Re: [PATCH 1/2] prctl: Add PR_SET_MM option description

[Cyrill Gorcunov <gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>]

On Sun, Apr 15, 2012 at 10:13:51PM +1200, Michael Kerrisk (man-pages) wrote:
> On Sun, Apr 15, 2012 at 6:54 PM, Cyrill Gorcunov <gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> wrote:
> > On Sun, Apr 15, 2012 at 03:48:18PM +1200, Michael Kerrisk (man-pages) wrote:
> >> Cyrill,
> >>
> >> While reviewing your patch to the prctl() manual page, I noticed the
> >> following code inkernel/sys.c::prctl_set_mm():
> >>
> >>         if (opt != PR_SET_MM_START_BRK && opt != PR_SET_MM_BRK) {
> >>                 /* It must be existing VMA */
> >>                 if (!vma || vma->vm_start > addr)
> >>                         goto out;
> >>         }
> >>
> >> At this point, the code causes an exit with error set to zero (i.e.,
> >> success). This looks unintended to me. Is the code correct? I suspect
> >> a return of -EFAULT or -ENOMEM is warranted.
> >
> > Hi Michael, yup, -EINVAL escaped (I think EFAULT or ENOMEM is not really
> > good here). I'll fix and send update. Thanks!
> 
> For what it's worth (I am no expert), it looks to me as though EFAULT
> or ENOMEM is more usual after a failed find_vma(). Furthermore, EINVAL
> is already heavily used, so not very informative as an error.

Would not ENOMEM be decoded by glibc as "no-memory" usually associated
with lack of free memory?

You know, I'm starting to think this checks for existing vmas might be
redundant completely. I tried to make this prctl codes to look somehow
close to elf loading procedure, where start|end_code/data do correspond
vmas loaded by kernel while parsing pt-load sections, but now I think
this is not needed, because start|end_code/data is not changed after
file is loaded but when we do checkpoint (and then restore) the program
map might be seriously changed (the program may unmap original areas,alocate
new vmas, put there code/data or whatever) thus there might be no correspond
vma at all when we setup this addresses for memory map (if only I'm not
missing something). So I guess I could drop this "existing vmas"
requirements. Need to think more :)

	Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html