Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

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

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


gorcunov at openvz

Feb 29, 2012, 4:23 AM

Post #1 of 11 (177 views)
Permalink
[PATCH 1/2] prctl: Add PR_SET_MM option description

Signed-off-by: Cyrill Gorcunov <gorcunov [at] openvz>
CC: Tejun Heo <tj [at] kernel>
CC: Pavel Emelyanov <xemul [at] parallels>
---
man2/prctl.2 | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/man2/prctl.2 b/man2/prctl.2
index effad2a..4d6244f 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -378,6 +378,110 @@ Return the current per-process machine check kill policy.
All unused
.BR prctl ()
arguments must be zero.
+.TP
+.BR PR_SET_MM " (since Linux 3.3)"
+Allows a user to modify certain kernel memory map descriptor fields
+of the calling process.
+Usually these fields are set by the kernel and dynamic loader (see
+.BR ld.so (8)
+for more information) and a regular application should not use this feature.
+Still there are cases such as self-modifying programs, where a program might
+find it useful to change its own memory map.
+The kernel must be built with
+.BR CONFIG_CHECKPOINT_RESTORE
+option turned on, otherwise this feature will not be accessible
+from a user space level.
+The calling process must have
+.BR CAP_SYS_ADMIN
+(see
+.BR capabilities (7)
+for details) capability granted.
+The value in
+.I arg2
+is one of the options below, while
+.I arg3
+provides a new value for this option.
+
+.BR PR_SET_MM_START_CODE
+to set the address above which program text can run.
+The corresponding memory area must be readable and executable,
+but not writable or shareable (see
+.BR mprotect (2)
+and
+.BR mmap (2)
+for more information).
+
+.BR PR_SET_MM_END_CODE
+to set the address below which program text can run.
+The corresponding memory area must be readable and executable,
+but not writable or shareable.
+
+.BR PR_SET_MM_START_DATA
+to set the address above which program data+bss is placed.
+The corresponding memory area must be readable and writable,
+but not executable or shareable.
+
+.B PR_SET_MM_END_DATA
+to set the address below which program data+bss is placed.
+The corresponding memory area must be readable and writable,
+but not executable or shareable.
+
+.BR PR_SET_MM_START_STACK
+to set the start address of the stack.
+The corresponding memory area must be readable and writable.
+
+.BR PR_SET_MM_START_BRK
+to set the address above which program heap can be expanded with
+.BR brk (2)
+call.
+The address must not be greater than ending address of
+the current program data segment, neither it may exceed
+resource limit for data (see
+.BR setrlimit (2)
+for more information).
+
+.BR PR_SET_MM_BRK
+to set the current
+.BR brk (2)
+value.
+The requirements for address are the same as for
+.BR PR_SET_MM_START_BRK
+option.
+
+.BR PR_SET_MM_ARG_START
+to set the address above which program command line is placed.
+
+.BR PR_SET_MM_ARG_END
+to set the address below which program command line is placed.
+
+.BR PR_SET_MM_ENV_START
+to set the address above which program environment is placed.
+
+.BR PR_SET_MM_ENV_END
+to set the address below which program environment is placed.
+
+The address passed with
+.BR PR_SET_MM_ARG_START ,
+.BR PR_SET_MM_ARG_END ,
+.BR PR_SET_MM_ENV_START ,
+.BR PR_SET_MM_ENV_END ,
+should belong to a process stack area, thus corresponding memory area
+must be readable, writable and (depending on the kernel
+configuration) has
+.BR MAP_GROWSDOWN
+attribute set (see
+.BR mmap (2)
+for details).
+
+.BR PR_SET_MM_AUXV
+to set a new auxiliary vector.
+The
+.I arg3
+argument should provide the address of the vector.
+The
+.I arg4
+is the size of the vector.
+.\"
.SH "RETURN VALUE"
On success,
.BR PR_GET_DUMPABLE ,
--
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mtk.manpages at gmail

Mar 6, 2012, 10:00 AM

Post #2 of 11 (189 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

Hi Cyrill,

Just a couple of comments for the moment.

On Thu, Mar 1, 2012 at 1:23 AM, Cyrill Gorcunov <gorcunov [at] openvz> wrote:
> Signed-off-by: Cyrill Gorcunov <gorcunov [at] openvz>
> CC: Tejun Heo <tj [at] kernel>
> CC: Pavel Emelyanov <xemul [at] parallels>
> ---
>  man2/prctl.2 |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index effad2a..4d6244f 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -378,6 +378,110 @@ Return the current per-process machine check kill policy.
>  All unused
>  .BR prctl ()
>  arguments must be zero.
> +.TP
> +.BR PR_SET_MM " (since Linux 3.3)"
> +Allows a user to modify certain kernel memory map descriptor fields
> +of the calling process.
> +Usually these fields are set by the kernel and dynamic loader (see
> +.BR ld.so (8)
> +for more information) and a regular application should not use this feature.
> +Still there are cases such as self-modifying programs, where a program might
> +find it useful to change its own memory map.

By the way, do you have a *simple* program that demonstrates some
usage of R_SET_MM?

> +The kernel must be built with
> +.BR CONFIG_CHECKPOINT_RESTORE
> +option turned on, otherwise this feature will not be accessible
> +from a user space level.
> +The calling process must have
> +.BR CAP_SYS_ADMIN
> +(see
> +.BR capabilities (7)
> +for details) capability granted.

As we discussed earlier (offlist), there are probably better choices
than the hugely overloaded CAP_SYS_ADMIN (see
http://man7.org/linux/man-pages/man7/capabilities.7.html). And if the
capability governing PR_SET_MM is to change, then it would be good to
do so before 3.3 is released. What are the plans on this point?

Cheers,

Michael


> +The value in
> +.I arg2
> +is one of the options below, while
> +.I arg3
> +provides a new value for this option.
> +
> +.BR PR_SET_MM_START_CODE
> +to set the address above which program text can run.
> +The corresponding memory area must be readable and executable,
> +but not writable or shareable (see
> +.BR mprotect (2)
> +and
> +.BR mmap (2)
> +for more information).
> +
> +.BR PR_SET_MM_END_CODE
> +to set the address below which program text can run.
> +The corresponding memory area must be readable and executable,
> +but not writable or shareable.
> +
> +.BR PR_SET_MM_START_DATA
> +to set the address above which program data+bss is placed.
> +The corresponding memory area must be readable and writable,
> +but not executable or shareable.
> +
> +.B PR_SET_MM_END_DATA
> +to set the address below which program data+bss is placed.
> +The corresponding memory area must be readable and writable,
> +but not executable or shareable.
> +
> +.BR PR_SET_MM_START_STACK
> +to set the start address of the stack.
> +The corresponding memory area must be readable and writable.
> +
> +.BR PR_SET_MM_START_BRK
> +to set the address above which program heap can be expanded with
> +.BR brk (2)
> +call.
> +The address must not be greater than ending address of
> +the current program data segment, neither it may exceed
> +resource limit for data (see
> +.BR setrlimit (2)
> +for more information).
> +
> +.BR PR_SET_MM_BRK
> +to set the current
> +.BR brk (2)
> +value.
> +The requirements for address are the same as for
> +.BR PR_SET_MM_START_BRK
> +option.
> +
> +.BR PR_SET_MM_ARG_START
> +to set the address above which program command line is placed.
> +
> +.BR PR_SET_MM_ARG_END
> +to set the address below which program command line is placed.
> +
> +.BR PR_SET_MM_ENV_START
> +to set the address above which program environment is placed.
> +
> +.BR PR_SET_MM_ENV_END
> +to set the address below which program environment is placed.
> +
> +The address passed with
> +.BR PR_SET_MM_ARG_START ,
> +.BR PR_SET_MM_ARG_END ,
> +.BR PR_SET_MM_ENV_START ,
> +.BR PR_SET_MM_ENV_END ,
> +should belong to a process stack area, thus corresponding memory area
> +must be readable, writable and (depending on the kernel
> +configuration) has
> +.BR MAP_GROWSDOWN
> +attribute set (see
> +.BR mmap (2)
> +for details).
> +
> +.BR PR_SET_MM_AUXV
> +to set a new auxiliary vector.
> +The
> +.I arg3
> +argument should provide the address of the vector.
> +The
> +.I arg4
> +is the size of the vector.
> +.\"
>  .SH "RETURN VALUE"
>  On success,
>  .BR PR_GET_DUMPABLE ,
> --
> 1.7.7.6
>



--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


gorcunov at openvz

Mar 6, 2012, 10:22 AM

Post #3 of 11 (171 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Wed, Mar 07, 2012 at 07:00:14AM +1300, Michael Kerrisk (man-pages) wrote:
> Hi Cyrill,
>
> Just a couple of comments for the moment.
>
> On Thu, Mar 1, 2012 at 1:23 AM, Cyrill Gorcunov <gorcunov [at] openvz> wrote:
> > Signed-off-by: Cyrill Gorcunov <gorcunov [at] openvz>
> > CC: Tejun Heo <tj [at] kernel>
> > CC: Pavel Emelyanov <xemul [at] parallels>
> > ---
> >  man2/prctl.2 |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 104 insertions(+), 0 deletions(-)
> >
> > diff --git a/man2/prctl.2 b/man2/prctl.2
> > index effad2a..4d6244f 100644
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -378,6 +378,110 @@ Return the current per-process machine check kill policy.
> >  All unused
> >  .BR prctl ()
> >  arguments must be zero.
> > +.TP
> > +.BR PR_SET_MM " (since Linux 3.3)"
> > +Allows a user to modify certain kernel memory map descriptor fields
> > +of the calling process.
> > +Usually these fields are set by the kernel and dynamic loader (see
> > +.BR ld.so (8)
> > +for more information) and a regular application should not use this feature.
> > +Still there are cases such as self-modifying programs, where a program might
> > +find it useful to change its own memory map.
>
> By the way, do you have a *simple* program that demonstrates some
> usage of R_SET_MM?

Hi Michael,

well, at moment we've only crtools itself which uses this facility,
so if we need complete standalone example I need to think about it.

>
> > +The kernel must be built with
> > +.BR CONFIG_CHECKPOINT_RESTORE
> > +option turned on, otherwise this feature will not be accessible
> > +from a user space level.
> > +The calling process must have
> > +.BR CAP_SYS_ADMIN
> > +(see
> > +.BR capabilities (7)
> > +for details) capability granted.
>
> As we discussed earlier (offlist), there are probably better choices
> than the hugely overloaded CAP_SYS_ADMIN (see
> http://man7.org/linux/man-pages/man7/capabilities.7.html). And if the
> capability governing PR_SET_MM is to change, then it would be good to
> do so before 3.3 is released. What are the plans on this point?
>

Yeah, I thought about changing it to CAP_SYS_RESOURCE here.
And I'll post a patch. The problem at moment that there another
two snippets needed for prctl -- ability to set new /proc/pid/exe
symlink and to obtaine clear-tid-address. So there is a discussion
now about symlink change. Once we finish with it -- i'll post
update for capability.

If you prefer to have it done earlier -- no problem, I'll cook
a patch today instead on top of everything we've already
merged into linux-next. What would you prefer?


Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mtk.manpages at gmail

Mar 6, 2012, 11:52 AM

Post #4 of 11 (170 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Wed, Mar 7, 2012 at 7:22 AM, Cyrill Gorcunov <gorcunov [at] openvz> wrote:
> On Wed, Mar 07, 2012 at 07:00:14AM +1300, Michael Kerrisk (man-pages) wrote:
>> Hi Cyrill,
>>
>> Just a couple of comments for the moment.
>>
>> On Thu, Mar 1, 2012 at 1:23 AM, Cyrill Gorcunov <gorcunov [at] openvz> wrote:
>> > Signed-off-by: Cyrill Gorcunov <gorcunov [at] openvz>
>> > CC: Tejun Heo <tj [at] kernel>
>> > CC: Pavel Emelyanov <xemul [at] parallels>
>> > ---
>> >  man2/prctl.2 |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 files changed, 104 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/man2/prctl.2 b/man2/prctl.2
>> > index effad2a..4d6244f 100644
>> > --- a/man2/prctl.2
>> > +++ b/man2/prctl.2
>> > @@ -378,6 +378,110 @@ Return the current per-process machine check kill policy.
>> >  All unused
>> >  .BR prctl ()
>> >  arguments must be zero.
>> > +.TP
>> > +.BR PR_SET_MM " (since Linux 3.3)"
>> > +Allows a user to modify certain kernel memory map descriptor fields
>> > +of the calling process.
>> > +Usually these fields are set by the kernel and dynamic loader (see
>> > +.BR ld.so (8)
>> > +for more information) and a regular application should not use this feature.
>> > +Still there are cases such as self-modifying programs, where a program might
>> > +find it useful to change its own memory map.
>>
>> By the way, do you have a *simple* program that demonstrates some
>> usage of R_SET_MM?
>
> Hi Michael,
>
> well, at moment we've only crtools itself which uses this facility,
> so if we need complete standalone example I need to think about it.
>
>>
>> > +The kernel must be built with
>> > +.BR CONFIG_CHECKPOINT_RESTORE
>> > +option turned on, otherwise this feature will not be accessible
>> > +from a user space level.
>> > +The calling process must have
>> > +.BR CAP_SYS_ADMIN
>> > +(see
>> > +.BR capabilities (7)
>> > +for details) capability granted.
>>
>> As we discussed earlier (offlist), there are probably better choices
>> than the hugely overloaded CAP_SYS_ADMIN (see
>> http://man7.org/linux/man-pages/man7/capabilities.7.html). And if the
>> capability governing PR_SET_MM is to change, then it would be good to
>> do so before 3.3 is released. What are the plans on this point?
>>
>
> Yeah, I thought about changing it to CAP_SYS_RESOURCE here.
> And I'll post a patch. The problem at moment that there another
> two snippets needed for prctl -- ability to set new /proc/pid/exe
> symlink and to obtaine clear-tid-address. So there is a discussion
> now about symlink change. Once we finish with it -- i'll post
> update for capability.
>
> If you prefer to have it done earlier -- no problem, I'll cook
> a patch today instead on top of everything we've already
> merged into linux-next. What would you prefer?

It would make sense if the capability requirements were finalized
before 3.3 is released. Changing them after 3.3 creates (at least a
little) pain for userspace.

Thanks,

Michael


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


gorcunov at openvz

Mar 6, 2012, 12:01 PM

Post #5 of 11 (170 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Wed, Mar 07, 2012 at 08:52:30AM +1300, Michael Kerrisk (man-pages) wrote:
...
> >
> > If you prefer to have it done earlier -- no problem, I'll cook
> > a patch today instead on top of everything we've already
> > merged into linux-next. What would you prefer?
>
> It would make sense if the capability requirements were finalized
> before 3.3 is released. Changing them after 3.3 creates (at least a
> little) pain for userspace.
>

OK. I'll update and send a patch out.

Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mtk.manpages at gmail

Mar 6, 2012, 12:07 PM

Post #6 of 11 (171 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Wed, Mar 7, 2012 at 9:01 AM, Cyrill Gorcunov <gorcunov [at] openvz> wrote:
> On Wed, Mar 07, 2012 at 08:52:30AM +1300, Michael Kerrisk (man-pages) wrote:
> ...
>> >
>> > If you prefer to have it done earlier -- no problem, I'll cook
>> > a patch today instead on top of everything we've already
>> > merged into linux-next. What would you prefer?
>>
>> It would make sense if the capability requirements were finalized
>> before 3.3 is released. Changing them after 3.3 creates (at least a
>> little) pain for userspace.
>>
>
> OK. I'll update and send a patch out.

Take a look at http://man7.org/linux/man-pages/man7/capabilities.7.html

The two most obvious alternatives are CAP_SYS_RESOURCE and
CAP_SYS_NICE. Maybe CAP_SYS_NICE is better? I say this because of the
(slight) similarity to existing operations in the CAP_SYS_NICE list.

Cheers,

Michael


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


gorcunov at openvz

Mar 6, 2012, 12:16 PM

Post #7 of 11 (170 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Wed, Mar 07, 2012 at 09:07:38AM +1300, Michael Kerrisk (man-pages) wrote:
> >>
> >> It would make sense if the capability requirements were finalized
> >> before 3.3 is released. Changing them after 3.3 creates (at least a
> >> little) pain for userspace.
> >>
> >
> > OK. I'll update and send a patch out.
>
> Take a look at http://man7.org/linux/man-pages/man7/capabilities.7.html
>
> The two most obvious alternatives are CAP_SYS_RESOURCE and
> CAP_SYS_NICE. Maybe CAP_SYS_NICE is better? I say this because of the
> (slight) similarity to existing operations in the CAP_SYS_NICE list.
>

Well, dunno Michael, CAP_SYS_RESOURCE looks a bit metter for me since
the process is modifying own 'resources' (in term of what it owns).
Maybe Andrew or Tejun have something to say?

Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mtk.manpages at gmail

Apr 14, 2012, 8:48 PM

Post #8 of 11 (149 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

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.

Cheers,

Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


gorcunov at openvz

Apr 14, 2012, 11:54 PM

Post #9 of 11 (146 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

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!

Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mtk.manpages at gmail

Apr 15, 2012, 3:13 AM

Post #10 of 11 (142 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

On Sun, Apr 15, 2012 at 6:54 PM, Cyrill Gorcunov <gorcunov [at] openvz> 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.

Cheers,

Michael


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


gorcunov at openvz

Apr 15, 2012, 3:10 PM

Post #11 of 11 (141 views)
Permalink
Re: [PATCH 1/2] prctl: Add PR_SET_MM option description [In reply to]

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 [at] openvz> 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-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.