| Message ID | 20230131155559.35800-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] proc: fix to check name length in proc_lookup_de() | expand |
Hi Andrew, Could you please take a look at this patchset? Or should I ping Alexey Dobriyan? Thanks, On 2023/1/31 23:55, 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. > > 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(-) > > diff --git a/fs/proc/generic.c b/fs/proc/generic.c > index 878d7c6db919..f547e9593a77 100644 > --- a/fs/proc/generic.c > +++ b/fs/proc/generic.c > @@ -245,6 +245,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) { > @@ -401,7 +404,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 > */
On Wed, 1 Feb 2023 21:01:14 +0800 Chao Yu <chao@kernel.org> wrote: > Hi Andrew, > > Could you please take a look at this patchset? Or should I ping > Alexey Dobriyan? > [patch 1/2]: Alexey wasn't keen on the v1 patch. What changed? [patch 2/2]: What is the benefit from this change?
On Thu, Feb 02, 2023 at 03:41:54PM -0800, Andrew Morton wrote: > On Wed, 1 Feb 2023 21:01:14 +0800 Chao Yu <chao@kernel.org> wrote: > > > Hi Andrew, > > > > Could you please take a look at this patchset? Or should I ping > > Alexey Dobriyan? > > > > [patch 1/2]: Alexey wasn't keen on the v1 patch. What changed? Nothing! /proc lived without this check for 30 years: int proc_match(int len,const char * name,struct proc_dir_entry * de) { register int same __asm__("ax"); if (!de || !de->low_ino) return 0; /* "" means "." ---> so paths like "/usr/lib//libc.a" work */ if (!len && (de->name[0]=='.') && (de->name[1]=='\0')) return 1; if (de->namelen != len) return 0; __asm__("cld\n\t" "repe ; cmpsb\n\t" "setz %%al" :"=a" (same) :"0" (0),"S" ((long) name),"D" ((long) de->name),"c" (len) :"cx","di","si"); return same; }
On 2023/2/3 7:41, Andrew Morton wrote: > [patch 1/2]: Alexey wasn't keen on the v1 patch. What changed? I replied to Alexey's comments on v1, but still didn't get any feedback, so I just rebase the code to last -next. Sorry, too rush to missed to add change log on v2. > > [patch 2/2]: What is the benefit from this change? Block size is mismatch in between results of stat() and statfs(), this patch tries to fix this issue. stat /proc/ File: /proc/ Size: 0 Blocks: 0 IO Block: 1024 directory Device: 14h/20d Inode: 1 Links: 310 Access: (0555/dr-xr-xr-x) Uid: ( 0/ root) Gid: ( 0/ root) Access: 2023-01-28 23:14:20.491937242 +0800 Modify: 2023-01-28 23:14:20.491937242 +0800 Change: 2023-01-28 23:14:20.491937242 +0800 Birth: - stat -f /proc/ File: "/proc/" ID: 0 Namelen: 255 Type: proc Block size: 4096 Fundamental block size: 4096 Blocks: Total: 0 Free: 0 Available: 0 Inodes: Total: 0 Free: 0
On 2023/2/5 20:21, Alexey Dobriyan wrote: > Nothing! /proc lived without this check for 30 years: Oh, I'm trying make error handling logic of proc's lookup() to be the same as other generic filesystem, it looks you don't think it's worth or necessary to do that, anyway, the change is not critial, let's ignore it, thank you for reviewing this patch. Thanks, > > int proc_match(int len,const char * name,struct proc_dir_entry * de) > { > register int same __asm__("ax"); > > if (!de || !de->low_ino) > return 0; > /* "" means "." ---> so paths like "/usr/lib//libc.a" work */ > if (!len && (de->name[0]=='.') && (de->name[1]=='\0')) > return 1; > if (de->namelen != len) > return 0; > __asm__("cld\n\t" > "repe ; cmpsb\n\t" > "setz %%al" > :"=a" (same) > :"0" (0),"S" ((long) name),"D" ((long) de->name),"c" (len) > :"cx","di","si"); > return same; > }
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index 878d7c6db919..f547e9593a77 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -245,6 +245,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) { @@ -401,7 +404,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(-)