[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250823015349.1650855-1-wangzijie1@honor.com>
Date: Sat, 23 Aug 2025 09:53:49 +0800
From: wangzijie <wangzijie1@...or.com>
To: <sbrivio@...hat.com>
CC: <adobriyan@...il.com>, <akpm@...ux-foundation.org>, <ast@...nel.org>,
<brauner@...nel.org>, <kirill.shutemov@...ux.intel.com>,
<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<passt-dev@...st.top>, <rick.p.edgecombe@...el.com>,
<viro@...iv.linux.org.uk>, <wangzijie1@...or.com>, <jirislaby@...nel.org>
Subject: Re: [PATCH] proc: Bring back lseek() operations for /proc/net entries
> Commit ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek
> as ones for proc_read_iter et.al") breaks lseek() for all /proc/net
> entries, as shown for instance by pasta(1), a user-mode network
> implementation using those entries to scan for bound ports:
>
> $ strace -e openat,lseek -e s=none pasta -- true
> [...]
> openat(AT_FDCWD, "/proc/net/tcp", O_RDONLY|O_CLOEXEC) = 12
> openat(AT_FDCWD, "/proc/net/tcp6", O_RDONLY|O_CLOEXEC) = 13
> lseek(12, 0, SEEK_SET) = -1 ESPIPE (Illegal seek)
> lseek() failed on /proc/net file: Illegal seek
> lseek(13, 0, SEEK_SET) = -1 ESPIPE (Illegal seek)
> lseek() failed on /proc/net file: Illegal seek
> openat(AT_FDCWD, "/proc/net/udp", O_RDONLY|O_CLOEXEC) = 14
> openat(AT_FDCWD, "/proc/net/udp6", O_RDONLY|O_CLOEXEC) = 15
> lseek(14, 0, SEEK_SET) = -1 ESPIPE (Illegal seek)
> lseek() failed on /proc/net file: Illegal seek
> lseek(15, 0, SEEK_SET) = -1 ESPIPE (Illegal seek)
> lseek() failed on /proc/net file: Illegal seek
> [...]
>
> That's because PROC_ENTRY_proc_lseek isn't set for /proc/net entries,
> and it's now mandatory for lseek(). In fact, flags aren't set at all
> for those entries because pde_set_flags() isn't called for them.
>
> As commit d919b33dafb3 ("proc: faster open/read/close with "permanent"
> files") introduced flags for procfs directory entries, along with the
> pde_set_flags() helper, they weren't relevant for /proc/net entries,
> so the lack of pde_set_flags() calls in proc_create_net_*() functions
> was harmless.
>
> Now that the calls are strictly needed for lseek() functionality,
> add them.
>
> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al")
> Signed-off-by: Stefano Brivio <sbrivio@...hat.com>
> ---
> fs/proc/generic.c | 2 +-
> fs/proc/internal.h | 1 +
> fs/proc/proc_net.c | 4 ++++
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 76e800e38c8f..57ec5e385d1b 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -561,7 +561,7 @@ struct proc_dir_entry *proc_create_reg(const char *name, umode_t mode,
> return p;
> }
>
> -static void pde_set_flags(struct proc_dir_entry *pde)
> +void pde_set_flags(struct proc_dir_entry *pde)
> {
> if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
> pde->flags |= PROC_ENTRY_PERMANENT;
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index e737401d7383..a358974f14d2 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -284,6 +284,7 @@ extern struct dentry *proc_lookup(struct inode *, struct dentry *, unsigned int)
> struct dentry *proc_lookup_de(struct inode *, struct dentry *, struct proc_dir_entry *);
> extern int proc_readdir(struct file *, struct dir_context *);
> int proc_readdir_de(struct file *, struct dir_context *, struct proc_dir_entry *);
> +void pde_set_flags(struct proc_dir_entry *pde);
>
> static inline void pde_get(struct proc_dir_entry *pde)
> {
> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index 52f0b75cbce2..20bc7481b02c 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -124,6 +124,7 @@ struct proc_dir_entry *proc_create_net_data(const char *name, umode_t mode,
> p->proc_ops = &proc_net_seq_ops;
> p->seq_ops = ops;
> p->state_size = state_size;
> + pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL_GPL(proc_create_net_data);
> @@ -170,6 +171,7 @@ struct proc_dir_entry *proc_create_net_data_write(const char *name, umode_t mode
> p->seq_ops = ops;
> p->state_size = state_size;
> p->write = write;
> + pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL_GPL(proc_create_net_data_write);
> @@ -217,6 +219,7 @@ struct proc_dir_entry *proc_create_net_single(const char *name, umode_t mode,
> pde_force_lookup(p);
> p->proc_ops = &proc_net_single_ops;
> p->single_show = show;
> + pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL_GPL(proc_create_net_single);
> @@ -261,6 +264,7 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
> p->proc_ops = &proc_net_single_ops;
> p->single_show = show;
> p->write = write;
> + pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL_GPL(proc_create_net_single_write);
> --
> 2.43.0
Hi Stefano,
Thanks for your patch, Lars reported this bug last week:
https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
Jiri suggested to make pde_set_flags() part of proc_register(). I think it can help
to avoid lack of pde_set_flags() calls in the future and make code clean.
I have submitted a patch:
https://lore.kernel.org/all/20250821105806.1453833-1-wangzijie1@honor.com
Powered by blists - more mailing lists