Hi Cyrill,
On 3/23/21 10:06 PM, Cyrill Gorcunov wrote:
[..]
> --- linux-tip.git.orig/kernel/sys.c
> +++ linux-tip.git/kernel/sys.c
> @@ -1961,6 +1961,30 @@ out:
> return error;
> }
>
> +static int copy_auxv_from_user(unsigned long *auxv, size_t auxv_size,
> +co
prctl(PR_SET_MM, PR_SET_MM_AUXV | PR_SET_MM_MAP, ...) copies user
provided auxiliary vector to kernel and saves it to mm::saved_auxv,
this involves same code in to places. Lets move it into one helper
instead.
When we copy data from user space we make sure that the vector ends
up with AT_NULL key
On Mon, Aug 25, 2014 at 02:08:43PM +0400, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> i
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call, in particular once new user namespace
is created capable(CAP_SYS_RESOURCE) no longer passes.
A approach is to eliminate CAP_SYS_RES
On Sat, Aug 23, 2014 at 09:29:15PM +0200, Oleg Nesterov wrote:
> On 08/23, Cyrill Gorcunov wrote:
> >
> > On Sat, Aug 23, 2014 at 05:32:22PM +0200, Oleg Nesterov wrote:
> > >
> > > And btw, where do you see RLIMIT_STACK in do_shmat() ?
> >
> > Indirectly, though start_stack pointer. We assign it in
On 08/23, Cyrill Gorcunov wrote:
>
> On Sat, Aug 23, 2014 at 05:32:22PM +0200, Oleg Nesterov wrote:
> >
> > And btw, where do you see RLIMIT_STACK in do_shmat() ?
>
> Indirectly, though start_stack pointer. We assign it in setup_arg_pages
> taking into
> account RLIMIT_STACK value, then do_shmat o
On Sat, Aug 23, 2014 at 05:32:22PM +0200, Oleg Nesterov wrote:
> > >
> > > Besides, it can't help anyway. cred_guard_mutex is per-process (not
> > > per-thread),
> > > suppose that a vfork()'ed child does prctl() while another thread reads
> > > the
> > > parent's /proc/pid/auxv.
> >
> > Then eit
On 08/23, Cyrill Gorcunov wrote:
>
> On Sat, Aug 23, 2014 at 03:30:01PM +0200, Oleg Nesterov wrote:
> >
> > On 08/23, Oleg Nesterov wrote:
> > >
> > > On 08/23, Cyrill Gorcunov wrote:
> > >
> > > > Looks like I need
> > > > to use cred_guard_mutex instead of task_lock here, no?
> > >
> > > Please d
On Sat, Aug 23, 2014 at 03:30:01PM +0200, Oleg Nesterov wrote:
> forgot to mention,
>
> On 08/23, Oleg Nesterov wrote:
> >
> > On 08/23, Cyrill Gorcunov wrote:
> >
> > > Looks like I need
> > > to use cred_guard_mutex instead of task_lock here, no?
> >
> > Please don't. First of all, it can't help
forgot to mention,
On 08/23, Oleg Nesterov wrote:
>
> On 08/23, Cyrill Gorcunov wrote:
>
> > Looks like I need
> > to use cred_guard_mutex instead of task_lock here, no?
>
> Please don't. First of all, it can't help because proc_pid_auxv() doesn't hold
> this lock. It does mm_access() which drops
On 08/23, Cyrill Gorcunov wrote:
>
> On Sat, Aug 23, 2014 at 01:53:02PM +0200, Oleg Nesterov wrote:
> > >
> > > It should protect from allocation/devetion/mergin of another vma. IOW when
> > > I lookup for vma I need to be sure it exist and won't disappear at least
> > > while I validate it.
> >
>
On Sat, Aug 23, 2014 at 01:53:02PM +0200, Oleg Nesterov wrote:
> >
> > It should protect from allocation/devetion/mergin of another vma. IOW when
> > I lookup for vma I need to be sure it exist and won't disappear at least
> > while I validate it.
>
> plus you need mmap_sem (at least for reading)
On 08/23, Cyrill Gorcunov wrote:
>
> On Fri, Aug 22, 2014 at 09:22:41PM +0200, Oleg Nesterov wrote:
> > Hi Cyrill,
> >
> > I think the patch is fine but I can't understand the usage of mmap_sem
> > and alloc_lock,
> >
> > > + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
> >
> >
On Fri, Aug 22, 2014 at 01:46:28PM -0700, Andrew Morton wrote:
> On Sat, 23 Aug 2014 00:38:09 +0400 Cyrill Gorcunov wrote:
>
> > >
> > > Or will we? What happens if we later decide that some additional field
> > > needs to be added? Do we version the interface? Add a new prctl()
> > > mode?
On Sat, 23 Aug 2014 00:38:09 +0400 Cyrill Gorcunov wrote:
> >
> > Or will we? What happens if we later decide that some additional field
> > needs to be added? Do we version the interface? Add a new prctl()
> > mode? Let's cook up a plan for that and at least add to changelog?
>
> I don't e
On Thu, Aug 21, 2014 at 11:49:12PM -0700, Andrew Morton wrote:
> > >
> > > But it's a bit hacky. Can anyone think of anything smarter?
> >
> > Looks good to me and not that hacky actually.
>
> Hacky :( I guess it's pretty safe because this is a userspace-visible
> structure so we'll never be ch
On Fri, Aug 22, 2014 at 09:22:41PM +0200, Oleg Nesterov wrote:
> Hi Cyrill,
>
> I think the patch is fine but I can't understand the usage of mmap_sem
> and alloc_lock,
>
> > + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
>
> OK, find_vma() needs mmap_sem. But otherwise, wh
Hi Cyrill,
I think the patch is fine but I can't understand the usage of mmap_sem
and alloc_lock,
> + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
OK, find_vma() needs mmap_sem. But otherwise, why this should be called
under down_read(&mm->mmap_sem) ? What this lock tries
On Fri, 22 Aug 2014 10:32:42 +0400 Cyrill Gorcunov wrote:
> > > + error |= __prctl_check_addr_space(start_code);
> > > + error |= __prctl_check_addr_space(end_code);
> > > + error |= __prctl_check_addr_space(start_data);
> > > + error |= __prctl_check_addr_space(end_data);
> > > + error |= __prct
On Thu, Aug 21, 2014 at 03:51:15PM -0700, Andrew Morton wrote:
...
> >
> > Still note that updating exe-file link now doesn't require sys-resource
> > capability anymore, after all there is no much profit in preventing setup
> > own file link (there are a number of ways to execute own code -- ptra
On Mon, 04 Aug 2014 21:22:59 +0400 Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> i
On Tue, Aug 05, 2014 at 12:08:53PM +0400, Andrew Vagin wrote:
> >
> > Signed-off-by: Cyrill Gorcunov
> > Cc: Kees Cook
> > Cc: Tejun Heo
> > Cc: Andrew Morton
> > Cc: Andrew Vagin
>
> Acked-by: Andrew Vagin
>
> I have tested this patch with criu. Everything work as expected.
Thanks Andrew
On Mon, Aug 04, 2014 at 09:22:59PM +0400, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> i
Quoting Cyrill Gorcunov (gorcu...@openvz.org):
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> is created capable(CAP_SYS_RE
Quoting Cyrill Gorcunov (gorcu...@openvz.org):
> Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
> it out of and rename the helper to prctl_set_mm_exe_file_locked.
> This will allow to reuse this function in a next patch.
>
> Signed-off-by: Cyrill Gorcunov
> Cc: Kees Cook
> Cc:
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call, in particular once new user namespace
is created capable(CAP_SYS_RESOURCE) no longer passes.
A approach is to eliminate CAP_SYS_RES
Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
it out of and rename the helper to prctl_set_mm_exe_file_locked.
This will allow to reuse this function in a next patch.
Signed-off-by: Cyrill Gorcunov
Cc: Kees Cook
Cc: Tejun Heo
Cc: Andrew Morton
Cc: Andrew Vagin
Cc: Eric W. B
On Thu, Jul 24, 2014 at 12:31:54PM -0700, Kees Cook wrote:
...
> > +
> > +#ifdef CONFIG_STACK_GROWSUP
> > + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> > + stack_vma->vm_end,
> > + prctl_map->start_stack, 0, 0))
> > +#else
> > + if (may_a
On Thu, Jul 24, 2014 at 9:47 AM, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> is created capable
On Thu, Jul 24, 2014 at 11:44:50AM -0700, Kees Cook wrote:
...
> >
> > The file can have a suid bit, so after executing it we may lose ability
> > to attach to it. To check that we can check that uid and gid is zero
> > in a current userns (local root).
> >
> > What else do we need to check?
>
> Y
On Thu, Jul 24, 2014 at 6:48 AM, Andrew Vagin wrote:
> On Tue, Jul 22, 2014 at 01:07:51PM -0700, Kees Cook wrote:
>> > - @exe_fd is referred from /proc/$pid/exe and when generating
>> >coredump. We uses prctl_set_mm_exe_file_locked helper to update
>> >this member, so exe-file link modifi
Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
it out of and rename the helper to prctl_set_mm_exe_file_locked.
This will allow to reuse this function in a next patch.
Signed-off-by: Cyrill Gorcunov
Cc: Kees Cook
Cc: Tejun Heo
Cc: Andrew Morton
Cc: Andrew Vagin
Cc: Eric W. B
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call, in particular once new user namespace
is created capable(CAP_SYS_RESOURCE) no longer passes.
A approach is to eliminate CAP_SYS_RES
On Thu, Jul 24, 2014 at 05:48:28PM +0400, Andrew Vagin wrote:
> On Tue, Jul 22, 2014 at 01:07:51PM -0700, Kees Cook wrote:
> > > - @exe_fd is referred from /proc/$pid/exe and when generating
> > >coredump. We uses prctl_set_mm_exe_file_locked helper to update
> > >this member, so exe-file
On Tue, Jul 22, 2014 at 01:07:51PM -0700, Kees Cook wrote:
> > - @exe_fd is referred from /proc/$pid/exe and when generating
> >coredump. We uses prctl_set_mm_exe_file_locked helper to update
> >this member, so exe-file link modification remains one-shot
> >action.
>
> Controlling exe
On Tue, Jul 22, 2014 at 01:07:51PM -0700, Kees Cook wrote:
> On Fri, Jul 11, 2014 at 10:36 AM, Cyrill Gorcunov wrote:
> > On Wed, Jul 09, 2014 at 07:06:04PM +0400, Cyrill Gorcunov wrote:
> >>
> >> Thanks a lot for comments, Kees! I tend to agre, leaving off the @prctl_map
> >> variable out of macr
this one should look better.
> ---
> From: Cyrill Gorcunov
> Subject: prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v2
>
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> p
x27;m not missing something
> in security aspects when time permits.
I suppse this one should look better.
---
From: Cyrill Gorcunov
Subject: prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v2
During development of c/r we've noticed that in case if we need to
support user namespaces
On Wed, Jul 09, 2014 at 07:53:10AM -0700, Kees Cook wrote:
...
> > +
> > + /*
> > +* Make sure the pairs are ordered.
> > +*/
> > +#define __prctl_check_order(__map, __m1, __m2)
> > \
> > + (unsigned long)__map->__m2 <= (unsigned long)__m
ite more descriptive comment since original "It takes
>> a pointer of prctl_mm_map structure which carries all members to be
>> updated" is too short.
>
> Here is a way more descriptove changelog I hope. Please poke me if
> more details needed, or something should be improv
o be
> updated" is too short.
Here is a way more descriptove changelog I hope. Please poke me if
more details needed, or something should be improved/changed and
etc.
---
From: Cyrill Gorcunov
Subject: prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation
During development of c/r we&#
On Tue, Jul 08, 2014 at 02:38:30PM -0700, Andrew Morton wrote:
> On Tue, 8 Jul 2014 23:08:49 +0400 Cyrill Gorcunov wrote:
>
> > Ping. Guys, any commens please?
>
> Well, allowing a process to modify pretty deep internals like this is
> always scary from a security point of view, and now we're re
On Tue, 8 Jul 2014 23:08:49 +0400 Cyrill Gorcunov wrote:
> Ping. Guys, any commens please?
Well, allowing a process to modify pretty deep internals like this is
always scary from a security point of view, and now we're removing
CAP_SYS_RESOURCE protections. Yikes. Convince me we aren't handing
On Thu, Jul 03, 2014 at 06:33:20PM +0400, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call.
>
> Current PR_SET_MM code forbids to
On Fri, Jul 04, 2014 at 11:52:56AM +0400, Andrew Vagin wrote:
> > +
> > +struct prctl_mm_map {
> > + unsigned long start_code;
>
> "unsigned long" has different sizes on x86_64 and x86, so a compat
> is required for x32 processes on x64 kernel.
Yes, good point. I think we can use u64/32 types
On Thu, Jul 03, 2014 at 06:33:20PM +0400, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call.
>
> Current PR_SET_MM code forbids to
On Thu, Jul 03, 2014 at 06:33:20PM +0400, Cyrill Gorcunov wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call.
Sigh, I updated changelog after I started writting t
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call.
Current PR_SET_MM code forbids to modify fields if no CAP_SYS_RESOURCE
granted, but rather relies on one who use this interface is pa
Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
it out of and rename the helper to prctl_set_mm_exe_file_locked.
This will allow to reuse this function in a next patch.
Signed-off-by: Cyrill Gorcunov
Cc: Kees Cook
Cc: Tejun Heo
Cc: Andrew Morton
Cc: Andrew Vagin
Cc: Eric W. B
to the local file-systems.
The code that does that was already included in the Linux kernel by
the CRIU group, in the form of "prctl(PR_SET_MM)", but prior to this
was enclosed within their private "#ifdef CONFIG_CHECKPOINT_RESTORE",
which is normally disabled.
It was not clea
to the local file-systems.
The code that does that was already included in the Linux kernel by
the CRIU group, in the form of "prctl(PR_SET_MM)", but prior to this
was enclosed within their private "#ifdef CONFIG_CHECKPOINT_RESTORE",
which is normally disabled.
It was not clea
On 02/19/2013 07:25 AM, Amnon Shiloh wrote:
> Steven Rostedt wrote:
>
>> If only you, or a few people are using it (ie. distros don't see a
>> need), then it will be up to you to make the changes.
>
> I believe that this functionality is of a general nature and is needed
> by many, not only by my
.." makes me sad.
>
> If/when this patch gets sent for real, please explain the reasons?
> AFAICT it permits the enabling of prctl(PR_SET_MM) in
> CONFIG_CHECKPOINT_RESTORE=n kernels. If that was indeed the intent,
> why?
>
>
--
To unsubscribe from this list: send the li
t; > Signed-off-by: Amnon Shiloh
>
> The "..." makes me sad.
>
> If/when this patch gets sent for real, please explain the reasons?
> AFAICT it permits the enabling of prctl(PR_SET_MM) in
> CONFIG_CHECKPOINT_RESTORE=n kernels. If that was indeed the intent,
> why?
S
, please explain the reasons?
AFAICT it permits the enabling of prctl(PR_SET_MM) in
CONFIG_CHECKPOINT_RESTORE=n kernels. If that was indeed the intent,
why?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.k
On Thu, Feb 21, 2013 at 07:03:58PM +1100, Amnon Shiloh wrote:
> Hi,
>
> Cyrill Gorcunov wrote:
>
> > Wouldn't the below do the same trick but eliminate OR in preproc code?
>
> Yes it would. I don't mind having it either way.
OK, lets wait for opinions and see if this approach is acceptable.
--
_FIELDS_SETTING
> + bool "Allow modifying per-process memory-region fields"
> + default y
> + help
> +Support "prctl(PR_SET_MM)" which allows applications to modify
> +the following in their "mm_struct":
> +
> +
bool "Allow modifying per-process memory-region fields"
+ default y
+ help
+ Support "prctl(PR_SET_MM)" which allows applications to modify
+ the following in their "mm_struct":
+
+ start_code, end_code, start_data, end_data
2 @@
If unsure, say N here.
+config MM_FIELDS_SETTING
+ bool "Allow modifying per-process memory-region fields"
+ default y
+ help
+ Support "prctl(PR_SET_MM)" which allows applications to modify
+ the following in their &qu
Hi Cyrill,
Cyrill Gorcunov wrote:
> I see. Thanks for explanation! Thus we need some new config option which would
> enable this prctl opcodes (y by default), in turn config-checkpoint-restore
> kconfig option need to select this feature if set. Sounds reasonable?
Yes, This would be one possible
On Wed, Feb 20, 2013 at 08:38:01PM +1100, Amnon Shiloh wrote:
> > I personally don't mind if this come become y by default, because it will
> > work for us.
>
> I don't mind either, to say the least, to have CONFIG_CHECKPOINT_RESTORE
> have a "default y" in "init/Kconfig" - that would solve my pro
Hi Cyrill,
Cyrill Gorcunov wrote:
> On Tue, Feb 19, 2013 at 05:25:31PM +1100, Amnon Shiloh wrote:
> >
> > To reconstruct Linux process(es), one must be able to restore all their
> > memory contents, states and registers to their original and consistent
> > values.
>
> I personally don't mind if
On Tue, Feb 19, 2013 at 05:25:31PM +1100, Amnon Shiloh wrote:
>
> To reconstruct Linux process(es), one must be able to restore all their
> memory contents, states and registers to their original and consistent
> values.
I personally don't mind if this come become y by default, because it will
wo
Steven Rostedt wrote:
> If only you, or a few people are using it (ie. distros don't see a
> need), then it will be up to you to make the changes.
I believe that this functionality is of a general nature and is needed
by many, not only by myself and by the CRIU group, but by all user-level
softwa
On Tue, 2013-02-19 at 03:33 +1100, Amnon Shiloh wrote:
> Yes, Randy Dunlap already raised this point, but I have no dealings with
> any particular Linux distribution or the right connections to chase them
> all, one by one - I develop generic software for the general Linux community,
> that is int
Steven Rostedt wrote:
> On Mon, 2013-02-18 at 12:39 +1100, Amnon Shiloh wrote:
> > Hello,
> >
> > The code in "kernel/sys.c" provides the "prctl(PR_SET_MM)" function,
> > which is the only way a process can set or modify the following 11
> >
On Mon, 2013-02-18 at 12:39 +1100, Amnon Shiloh wrote:
> Hello,
>
> The code in "kernel/sys.c" provides the "prctl(PR_SET_MM)" function,
> which is the only way a process can set or modify the following 11
> per-process fields:
>
> start_code, end_c
On 02/17/13 17:39, Amnon Shiloh wrote:
> Hello,
>
> The code in "kernel/sys.c" provides the "prctl(PR_SET_MM)" function,
> which is the only way a process can set or modify the following 11
> per-process fields:
>
> start_code, end_c
Hello,
The code in "kernel/sys.c" provides the "prctl(PR_SET_MM)" function,
which is the only way a process can set or modify the following 11
per-process fields:
start_code, end_code, start_data, end_data, start_brk, brk,
start_stack, arg_start, arg_end
69 matches
Mail list logo
