| Message ID | 155790813764.2995.13706842444028749629.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: fix protection of mm_struct fields in get_cmdline() | expand |
On Wed, May 15, 2019 at 11:15:37AM +0300, Konstantin Khlebnikov wrote: > Since commit 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start| > end and env_start|end in mm_struct") related mm fields are protected with > separate spinlock and mmap_sem held for read is not enough for protection. > > Fixes: 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct") > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> This was already addressed by [1]? [1] https://patchwork.kernel.org/patch/10923003/ > --- > mm/util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/util.c b/mm/util.c > index e2e4f8c3fa12..540e7c157cf2 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -717,12 +717,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; > >
On 15.05.2019 11:22, Oscar Salvador wrote: > On Wed, May 15, 2019 at 11:15:37AM +0300, Konstantin Khlebnikov wrote: >> Since commit 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start| >> end and env_start|end in mm_struct") related mm fields are protected with >> separate spinlock and mmap_sem held for read is not enough for protection. >> >> Fixes: 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct") >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > > This was already addressed by [1]? Yep. > > [1] https://patchwork.kernel.org/patch/10923003/ > >> --- >> mm/util.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/mm/util.c b/mm/util.c >> index e2e4f8c3fa12..540e7c157cf2 100644 >> --- a/mm/util.c >> +++ b/mm/util.c >> @@ -717,12 +717,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; >> >> >
Hi, Konstantin, On 15.05.2019 11:15, Konstantin Khlebnikov wrote: > Since commit 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start| > end and env_start|end in mm_struct") related mm fields are protected with > separate spinlock and mmap_sem held for read is not enough for protection. > > Fixes: 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct") > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> is this already fixed in Michal's series: https://lkml.org/lkml/2019/5/2/422 ? Thanks, Kirill > --- > mm/util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/util.c b/mm/util.c > index e2e4f8c3fa12..540e7c157cf2 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -717,12 +717,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; > >
diff --git a/mm/util.c b/mm/util.c index e2e4f8c3fa12..540e7c157cf2 100644 --- a/mm/util.c +++ b/mm/util.c @@ -717,12 +717,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;
Since commit 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start| end and env_start|end in mm_struct") related mm fields are protected with separate spinlock and mmap_sem held for read is not enough for protection. Fixes: 88aa7cc688d4 ("mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct") Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- mm/util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)