[1/2] proc: fix to check name length in proc_lookup_de()

  __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(-)
  

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;
> }