| Message ID | 20230110152112.1119517-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | proc: fix to check name length in proc_lookup_de() | expand |
On Tue, Jan 10, 2023 at 11:21:12PM +0800, Chao Yu wrote: > __proc_create() has limited dirent's max name length with 255, let's > add this limitation in proc_lookup_de(), so that it can return > -ENAMETOOLONG correctly instead of -ENOENT when stating a file which > has out-of-range name length. Both returns are correct and this is trading one errno for another. > --- a/fs/proc/generic.c > +++ b/fs/proc/generic.c > @@ -246,6 +246,9 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry, > { > struct inode *inode; > > + if (dentry->d_name.len > PROC_NAME_LEN) > + return ERR_PTR(-ENAMETOOLONG);
On 2023/1/11 2:01, Alexey Dobriyan wrote: > On Tue, Jan 10, 2023 at 11:21:12PM +0800, Chao Yu wrote: >> __proc_create() has limited dirent's max name length with 255, let's >> add this limitation in proc_lookup_de(), so that it can return >> -ENAMETOOLONG correctly instead of -ENOENT when stating a file which >> has out-of-range name length. > > Both returns are correct and this is trading one errno for another. Oh, but it looks ENOENT is a little bit ambiguity, it may indicate file name length is valid for procfs, but the entry is not exist. This change is trying to make lookup logic keeping align w/ most other filesystems' behavior. Also it can avoid running into unneeded lookup logic in proc_lookup_de() for such ENAMETOOLONG case. How do you think? :) Thanks, > >> --- a/fs/proc/generic.c >> +++ b/fs/proc/generic.c >> @@ -246,6 +246,9 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry, >> { >> struct inode *inode; >> >> + if (dentry->d_name.len > PROC_NAME_LEN) >> + return ERR_PTR(-ENAMETOOLONG);
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index 587b91d9d998..5f52f20d5ed1 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -246,6 +246,9 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry, { struct inode *inode; + if (dentry->d_name.len > PROC_NAME_LEN) + return ERR_PTR(-ENAMETOOLONG); + read_lock(&proc_subdir_lock); de = pde_subdir_find(de, dentry->d_name.name, dentry->d_name.len); if (de) { @@ -402,7 +405,7 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, goto out; qstr.name = fn; qstr.len = strlen(fn); - if (qstr.len == 0 || qstr.len >= 256) { + if (qstr.len == 0 || qstr.len > PROC_NAME_LEN) { WARN(1, "name len %u\n", qstr.len); return NULL; } diff --git a/fs/proc/internal.h b/fs/proc/internal.h index b701d0207edf..7611bc684d9e 100644 --- a/fs/proc/internal.h +++ b/fs/proc/internal.h @@ -142,6 +142,9 @@ unsigned name_to_int(const struct qstr *qstr); /* Worst case buffer size needed for holding an integer. */ #define PROC_NUMBUF 13 +/* Max name length of procfs dirent */ +#define PROC_NAME_LEN 255 + /* * array.c */
__proc_create() has limited dirent's max name length with 255, let's add this limitation in proc_lookup_de(), so that it can return -ENAMETOOLONG correctly instead of -ENOENT when stating a file which has out-of-range name length. Signed-off-by: Chao Yu <chao@kernel.org> --- fs/proc/generic.c | 5 ++++- fs/proc/internal.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)