| Message ID | 20190417120347.15397-1-mkoutny@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: get_cmdline use arg_lock instead of mmap_sem | expand |
On Wed 17-04-19 14:03:47, Michal Koutny wrote: > The commit a3b609ef9f8b ("proc read mm's {arg,env}_{start,end} with mmap > semaphore taken.") added synchronization of reading argument/environment > boundaries under mmap_sem. Later commit 88aa7cc688d4 ("mm: introduce > arg_lock to protect arg_start|end and env_start|end in mm_struct") > avoided the coarse use of mmap_sem in similar situations. > > get_cmdline can also use arg_lock instead of mmap_sem when it reads the > boundaries. Don't we need to use the lock in prctl_set_mm as well then? > Signed-off-by: Michal Koutný <mkoutny@suse.com> > --- > mm/util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/util.c b/mm/util.c > index d559bde497a9..568575cceefc 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -758,12 +758,12 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen) > if (!mm->arg_end) > goto out_mm; /* Shh! No looking before we're done */ > > - down_read(&mm->mmap_sem); > + spin_lock(&mm->arg_lock); > arg_start = mm->arg_start; > arg_end = mm->arg_end; > env_start = mm->env_start; > env_end = mm->env_end; > - up_read(&mm->mmap_sem); > + spin_unlock(&mm->arg_lock); > > len = arg_end - arg_start; > > -- > 2.16.4
On Wed, Apr 17, 2019 at 03:41:52PM +0200, Michal Hocko <mhocko@kernel.org> wrote:
> Don't we need to use the lock in prctl_set_mm as well then?
Correct. The patch alone just moves the race from
get_cmdline/prctl_set_mm_map to get_cmdline/prctl_set_mm.
arg_lock could be used in prctl_set_mm but the better idea (IMO) is
complete removal of that code in favor of prctl_set_mm_map [1].
Michal
[1] https://lore.kernel.org/lkml/20180405182651.GM15783@uranus.lan/
On Wed 17-04-19 16:41:42, Michal Koutny wrote: > On Wed, Apr 17, 2019 at 03:41:52PM +0200, Michal Hocko <mhocko@kernel.org> wrote: > > Don't we need to use the lock in prctl_set_mm as well then? > > Correct. The patch alone just moves the race from > get_cmdline/prctl_set_mm_map to get_cmdline/prctl_set_mm. > > arg_lock could be used in prctl_set_mm but the better idea (IMO) is > complete removal of that code in favor of prctl_set_mm_map [1]. Ohh, I have missed that patch. As long as both are merged together then no objections from me and you can add Acked-by: Michal Hocko <mhocko@suse.com> > Michal > > [1] https://lore.kernel.org/lkml/20180405182651.GM15783@uranus.lan/
diff --git a/mm/util.c b/mm/util.c index d559bde497a9..568575cceefc 100644 --- a/mm/util.c +++ b/mm/util.c @@ -758,12 +758,12 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen) if (!mm->arg_end) goto out_mm; /* Shh! No looking before we're done */ - down_read(&mm->mmap_sem); + spin_lock(&mm->arg_lock); arg_start = mm->arg_start; arg_end = mm->arg_end; env_start = mm->env_start; env_end = mm->env_end; - up_read(&mm->mmap_sem); + spin_unlock(&mm->arg_lock); len = arg_end - arg_start;
The commit a3b609ef9f8b ("proc read mm's {arg,env}_{start,end} with mmap semaphore taken.") added synchronization of reading argument/environment boundaries under mmap_sem. Later commit 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct") avoided the coarse use of mmap_sem in similar situations. get_cmdline can also use arg_lock instead of mmap_sem when it reads the boundaries. Signed-off-by: Michal Koutný <mkoutny@suse.com> --- mm/util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)