* [PATCH] fs: proc: fix error return code of proc_map_files_readdir()
@ 2021-03-09 9:55 Jia-Ju Bai
2021-03-09 18:30 ` Eric Biggers
0 siblings, 1 reply; 4+ messages in thread
From: Jia-Ju Bai @ 2021-03-09 9:55 UTC (permalink / raw)
To: adobriyan, christian, ebiederm, akpm, keescook, gladkov.alexey,
walken, bernd.edlinger, avagin, deller
Cc: linux-kernel, linux-fsdevel, Jia-Ju Bai
When get_task_mm() returns NULL to mm, no error return code of
proc_map_files_readdir() is assigned.
To fix this bug, ret is assigned with -ENOENT in this case.
Fixes: f0c3b5093add ("[readdir] convert procfs")
Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
fs/proc/base.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 3851bfcdba56..254cc6ac65fb 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2332,8 +2332,10 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
goto out_put_task;
mm = get_task_mm(task);
- if (!mm)
+ if (!mm) {
+ ret = -ENOENT;
goto out_put_task;
+ }
ret = mmap_read_lock_killable(mm);
if (ret) {
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] fs: proc: fix error return code of proc_map_files_readdir()
2021-03-09 9:55 [PATCH] fs: proc: fix error return code of proc_map_files_readdir() Jia-Ju Bai
@ 2021-03-09 18:30 ` Eric Biggers
2021-03-09 19:05 ` Alexey Dobriyan
0 siblings, 1 reply; 4+ messages in thread
From: Eric Biggers @ 2021-03-09 18:30 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: adobriyan, christian, ebiederm, akpm, keescook, gladkov.alexey,
walken, bernd.edlinger, avagin, deller, linux-kernel,
linux-fsdevel
On Tue, Mar 09, 2021 at 01:55:27AM -0800, Jia-Ju Bai wrote:
> When get_task_mm() returns NULL to mm, no error return code of
> proc_map_files_readdir() is assigned.
> To fix this bug, ret is assigned with -ENOENT in this case.
>
> Fixes: f0c3b5093add ("[readdir] convert procfs")
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> fs/proc/base.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 3851bfcdba56..254cc6ac65fb 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2332,8 +2332,10 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
> goto out_put_task;
>
> mm = get_task_mm(task);
> - if (!mm)
> + if (!mm) {
> + ret = -ENOENT;
> goto out_put_task;
> + }
>
> ret = mmap_read_lock_killable(mm);
Is there something in particular that makes you think that returning ENOENT is
the correct behavior in this case? Try 'ls /proc/$pid/map_files' where pid is a
kernel thread; it's an empty directory, which is probably intentional. Your
patch would change reading the directory to fail with ENOENT.
- Eric
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fs: proc: fix error return code of proc_map_files_readdir()
2021-03-09 18:30 ` Eric Biggers
@ 2021-03-09 19:05 ` Alexey Dobriyan
2021-03-10 1:11 ` Jia-Ju Bai
0 siblings, 1 reply; 4+ messages in thread
From: Alexey Dobriyan @ 2021-03-09 19:05 UTC (permalink / raw)
To: Eric Biggers
Cc: Jia-Ju Bai, christian, ebiederm, akpm, keescook, gladkov.alexey,
walken, bernd.edlinger, avagin, deller, linux-kernel,
linux-fsdevel
On Tue, Mar 09, 2021 at 10:30:23AM -0800, Eric Biggers wrote:
> On Tue, Mar 09, 2021 at 01:55:27AM -0800, Jia-Ju Bai wrote:
> > When get_task_mm() returns NULL to mm, no error return code of
> > proc_map_files_readdir() is assigned.
> > To fix this bug, ret is assigned with -ENOENT in this case.
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -2332,8 +2332,10 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
> > goto out_put_task;
> >
> > mm = get_task_mm(task);
> > - if (!mm)
> > + if (!mm) {
> > + ret = -ENOENT;
> > goto out_put_task;
> > + }
> >
> > ret = mmap_read_lock_killable(mm);
>
> Is there something in particular that makes you think that returning ENOENT is
> the correct behavior in this case? Try 'ls /proc/$pid/map_files' where pid is a
> kernel thread; it's an empty directory, which is probably intentional. Your
> patch would change reading the directory to fail with ENOENT.
Yes. 0 from readdir means "no more stuff", not an error.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fs: proc: fix error return code of proc_map_files_readdir()
2021-03-09 19:05 ` Alexey Dobriyan
@ 2021-03-10 1:11 ` Jia-Ju Bai
0 siblings, 0 replies; 4+ messages in thread
From: Jia-Ju Bai @ 2021-03-10 1:11 UTC (permalink / raw)
To: Alexey Dobriyan, Eric Biggers
Cc: christian, ebiederm, akpm, keescook, gladkov.alexey, walken,
bernd.edlinger, avagin, deller, linux-kernel, linux-fsdevel
On 2021/3/10 3:05, Alexey Dobriyan wrote:
> On Tue, Mar 09, 2021 at 10:30:23AM -0800, Eric Biggers wrote:
>
>>> --- a/fs/proc/base.c
>>> +++ b/fs/proc/base.c
>>> @@ -2332,8 +2332,10 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
>>> goto out_put_task;
>>>
>>> mm = get_task_mm(task);
>>> - if (!mm)
>>> + if (!mm) {
>>> + ret = -ENOENT;
>>> goto out_put_task;
>>> + }
>>>
>>> ret = mmap_read_lock_killable(mm);
>> Is there something in particular that makes you think that returning ENOENT is
>> the correct behavior in this case? Try 'ls /proc/$pid/map_files' where pid is a
>> kernel thread; it's an empty directory, which is probably intentional. Your
>> patch would change reading the directory to fail with ENOENT.
> Yes. 0 from readdir means "no more stuff", not an error.
Thanks for your reply and explanation.
I am sorry for the false report...
Best wishes,
Jia-Ju Bai
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-10 1:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-09 9:55 [PATCH] fs: proc: fix error return code of proc_map_files_readdir() Jia-Ju Bai
2021-03-09 18:30 ` Eric Biggers
2021-03-09 19:05 ` Alexey Dobriyan
2021-03-10 1:11 ` Jia-Ju Bai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).