| Message ID | 20220914064935.7851-1-xueshuai@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm,hwpoison: check mm when killing accessing process | expand |
On Wed, 14 Sep 2022 14:49:35 +0800 Shuai Xue <xueshuai@linux.alibaba.com> wrote: > The GHES code calls memory_failure_queue() from IRQ context to queue work > into workqueue and schedule it on the current CPU. Then the work is > processed in memory_failure_work_func() by kworker and calls > memory_failure(). > > When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send > SIGBUS with error virutal address") make memory_failure() call > kill_accessing_process() that: > > - holds mmap locking of current->mm > - does pagetable walk to find the error virtual address > - and sends SIGBUS to the current process with error info. > > However, the mm of kworker is not valid. Therefore, check mm when killing > accessing process. Thanks. When fixing a bug, please always describe the user-visible effects of tha bug. I'm thinking "null pointer deref crashes the kernel". > Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> I'll add cc:stable. > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -743,6 +743,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, > }; > priv.tk.tsk = p; > > + if (!p->mm) > + return -EFAULT; > + > mmap_read_lock(p->mm); > ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops, > (void *)&priv); > @@ -751,6 +754,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, > else > ret = 0; > mmap_read_unlock(p->mm); > + > return ret > 0 ? -EHWPOISON : -EFAULT; > } This is an unrelated change which doesn't appear to match the style in memory-failure.c, so I'll drop this hunk.
在 2022/9/15 AM6:35, Andrew Morton 写道: > On Wed, 14 Sep 2022 14:49:35 +0800 Shuai Xue <xueshuai@linux.alibaba.com> wrote: > >> The GHES code calls memory_failure_queue() from IRQ context to queue work >> into workqueue and schedule it on the current CPU. Then the work is >> processed in memory_failure_work_func() by kworker and calls >> memory_failure(). >> >> When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send >> SIGBUS with error virutal address") make memory_failure() call >> kill_accessing_process() that: >> >> - holds mmap locking of current->mm >> - does pagetable walk to find the error virtual address >> - and sends SIGBUS to the current process with error info. >> >> However, the mm of kworker is not valid. Therefore, check mm when killing >> accessing process. > > Thanks. > > When fixing a bug, please always describe the user-visible effects of > tha bug. I'm thinking "null pointer deref crashes the kernel". Got it. Thank you :) > >> Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > > I'll add cc:stable. Thanks. > >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -743,6 +743,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, >> }; >> priv.tk.tsk = p; >> >> + if (!p->mm) >> + return -EFAULT; >> + >> mmap_read_lock(p->mm); >> ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops, >> (void *)&priv); >> @@ -751,6 +754,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, >> else >> ret = 0; >> mmap_read_unlock(p->mm); >> + >> return ret > 0 ? -EHWPOISON : -EFAULT; >> } > > This is an unrelated change which doesn't appear to match the style in > memory-failure.c, so I'll drop this hunk. I see, thanks. Cheers, Shuai
On 2022/9/14 14:49, Shuai Xue wrote: > The GHES code calls memory_failure_queue() from IRQ context to queue work > into workqueue and schedule it on the current CPU. Then the work is > processed in memory_failure_work_func() by kworker and calls > memory_failure(). > > When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send > SIGBUS with error virutal address") make memory_failure() call > kill_accessing_process() that: > > - holds mmap locking of current->mm > - does pagetable walk to find the error virtual address > - and sends SIGBUS to the current process with error info. > > However, the mm of kworker is not valid. Therefore, check mm when killing > accessing process. > > Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Thanks for fixing. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thanks, Miaohe Lin > --- > mm/memory-failure.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 14439806b5ef..7553917ce820 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -743,6 +743,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, > }; > priv.tk.tsk = p; > > + if (!p->mm) > + return -EFAULT; > + > mmap_read_lock(p->mm); > ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops, > (void *)&priv); > @@ -751,6 +754,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, > else > ret = 0; > mmap_read_unlock(p->mm); > + > return ret > 0 ? -EHWPOISON : -EFAULT; > } > >
On Wed, Sep 14, 2022 at 02:49:35PM +0800, Shuai Xue wrote: > The GHES code calls memory_failure_queue() from IRQ context to queue work > into workqueue and schedule it on the current CPU. Then the work is > processed in memory_failure_work_func() by kworker and calls > memory_failure(). > > When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send > SIGBUS with error virutal address") make memory_failure() call > kill_accessing_process() that: > > - holds mmap locking of current->mm > - does pagetable walk to find the error virtual address > - and sends SIGBUS to the current process with error info. > > However, the mm of kworker is not valid. Therefore, check mm when killing > accessing process. > > Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Thank you for the patch, the fix itself makes sense to me. I agree with this patch with applying comments from Andrew. Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
在 2022/9/15 AM9:45, Miaohe Lin 写道: > On 2022/9/14 14:49, Shuai Xue wrote: >> The GHES code calls memory_failure_queue() from IRQ context to queue work >> into workqueue and schedule it on the current CPU. Then the work is >> processed in memory_failure_work_func() by kworker and calls >> memory_failure(). >> >> When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send >> SIGBUS with error virutal address") make memory_failure() call >> kill_accessing_process() that: >> >> - holds mmap locking of current->mm >> - does pagetable walk to find the error virtual address >> - and sends SIGBUS to the current process with error info. >> >> However, the mm of kworker is not valid. Therefore, check mm when killing >> accessing process. >> >> Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > > Thanks for fixing. > > Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> > > Thanks, > Miaohe Lin Thank you for your review. Cheers, Shuai > > >> --- >> mm/memory-failure.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index 14439806b5ef..7553917ce820 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -743,6 +743,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, >> }; >> priv.tk.tsk = p; >> >> + if (!p->mm) >> + return -EFAULT; >> + >> mmap_read_lock(p->mm); >> ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops, >> (void *)&priv); >> @@ -751,6 +754,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, >> else >> ret = 0; >> mmap_read_unlock(p->mm); >> + >> return ret > 0 ? -EHWPOISON : -EFAULT; >> } >> >>
在 2022/9/15 AM9:48, Naoya Horiguchi 写道: > On Wed, Sep 14, 2022 at 02:49:35PM +0800, Shuai Xue wrote: >> The GHES code calls memory_failure_queue() from IRQ context to queue work >> into workqueue and schedule it on the current CPU. Then the work is >> processed in memory_failure_work_func() by kworker and calls >> memory_failure(). >> >> When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send >> SIGBUS with error virutal address") make memory_failure() call >> kill_accessing_process() that: >> >> - holds mmap locking of current->mm >> - does pagetable walk to find the error virtual address >> - and sends SIGBUS to the current process with error info. >> >> However, the mm of kworker is not valid. Therefore, check mm when killing >> accessing process. >> >> Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > > Thank you for the patch, the fix itself makes sense to me. > I agree with this patch with applying comments from Andrew. > > Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> Thank you for prompt review. Cheers, Shuai
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 14439806b5ef..7553917ce820 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -743,6 +743,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, }; priv.tk.tsk = p; + if (!p->mm) + return -EFAULT; + mmap_read_lock(p->mm); ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops, (void *)&priv); @@ -751,6 +754,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn, else ret = 0; mmap_read_unlock(p->mm); + return ret > 0 ? -EHWPOISON : -EFAULT; }
The GHES code calls memory_failure_queue() from IRQ context to queue work into workqueue and schedule it on the current CPU. Then the work is processed in memory_failure_work_func() by kworker and calls memory_failure(). When a page is already poisoned, commit a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") make memory_failure() call kill_accessing_process() that: - holds mmap locking of current->mm - does pagetable walk to find the error virtual address - and sends SIGBUS to the current process with error info. However, the mm of kworker is not valid. Therefore, check mm when killing accessing process. Fixes: a3f5d80ea401 ("mm,hwpoison: send SIGBUS with error virutal address") Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- mm/memory-failure.c | 4 ++++ 1 file changed, 4 insertions(+)