* [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files
@ 2025-08-18 12:31 wangzijie
2025-08-21 4:28 ` Andrew Morton
2025-08-21 7:35 ` Christian Brauner
0 siblings, 2 replies; 4+ messages in thread
From: wangzijie @ 2025-08-18 12:31 UTC (permalink / raw)
To: akpm, viro, adobriyan, rick.p.edgecombe, ast, k.shutemov,
jirislaby, linux-fsdevel
Cc: polynomial-c, gregkh, stable, regressions, wangzijie
To avoid potential UAF issues during module removal races, we use pde_set_flags()
to save proc_ops flags in PDE itself before proc_register(), and then use
pde_has_proc_*() helpers instead of directly dereferencing pde->proc_ops->*.
However, the pde_set_flags() call was missing when creating net related proc files.
This omission caused incorrect behavior which FMODE_LSEEK was being cleared
inappropriately in proc_reg_open() for net proc files. Lars reported it in this link[1].
Fix this by ensuring pde_set_flags() is called when register proc entry, and add
NULL check for proc_ops in pde_set_flags().
[1]: https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al)
Cc: stable@vger.kernel.org
Reported-by: Lars Wendler <polynomial-c@gmx.de>
Signed-off-by: wangzijie <wangzijie1@honor.com>
---
v2:
- followed by Jiri's suggestion to refractor code and reformat commit message
---
fs/proc/generic.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 76e800e38..003031839 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -367,6 +367,23 @@ static const struct inode_operations proc_dir_inode_operations = {
.setattr = proc_notify_change,
};
+static void pde_set_flags(struct proc_dir_entry *pde)
+{
+ if (!pde->proc_ops)
+ return;
+
+ if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
+ pde->flags |= PROC_ENTRY_PERMANENT;
+ if (pde->proc_ops->proc_read_iter)
+ pde->flags |= PROC_ENTRY_proc_read_iter;
+#ifdef CONFIG_COMPAT
+ if (pde->proc_ops->proc_compat_ioctl)
+ pde->flags |= PROC_ENTRY_proc_compat_ioctl;
+#endif
+ if (pde->proc_ops->proc_lseek)
+ pde->flags |= PROC_ENTRY_proc_lseek;
+}
+
/* returns the registered entry, or frees dp and returns NULL on failure */
struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
struct proc_dir_entry *dp)
@@ -374,6 +391,8 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
if (proc_alloc_inum(&dp->low_ino))
goto out_free_entry;
+ pde_set_flags(dp);
+
write_lock(&proc_subdir_lock);
dp->parent = dir;
if (pde_subdir_insert(dir, dp) == false) {
@@ -561,20 +580,6 @@ 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)
-{
- if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
- pde->flags |= PROC_ENTRY_PERMANENT;
- if (pde->proc_ops->proc_read_iter)
- pde->flags |= PROC_ENTRY_proc_read_iter;
-#ifdef CONFIG_COMPAT
- if (pde->proc_ops->proc_compat_ioctl)
- pde->flags |= PROC_ENTRY_proc_compat_ioctl;
-#endif
- if (pde->proc_ops->proc_lseek)
- pde->flags |= PROC_ENTRY_proc_lseek;
-}
-
struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
struct proc_dir_entry *parent,
const struct proc_ops *proc_ops, void *data)
@@ -585,7 +590,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
if (!p)
return NULL;
p->proc_ops = proc_ops;
- pde_set_flags(p);
return proc_register(parent, p);
}
EXPORT_SYMBOL(proc_create_data);
@@ -636,7 +640,6 @@ struct proc_dir_entry *proc_create_seq_private(const char *name, umode_t mode,
p->proc_ops = &proc_seq_ops;
p->seq_ops = ops;
p->state_size = state_size;
- pde_set_flags(p);
return proc_register(parent, p);
}
EXPORT_SYMBOL(proc_create_seq_private);
@@ -667,7 +670,6 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
return NULL;
p->proc_ops = &proc_single_ops;
p->single_show = show;
- pde_set_flags(p);
return proc_register(parent, p);
}
EXPORT_SYMBOL(proc_create_single_data);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files
2025-08-18 12:31 [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files wangzijie
@ 2025-08-21 4:28 ` Andrew Morton
2025-08-21 7:35 ` Christian Brauner
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2025-08-21 4:28 UTC (permalink / raw)
To: wangzijie
Cc: viro, adobriyan, rick.p.edgecombe, ast, k.shutemov, jirislaby,
linux-fsdevel, polynomial-c, gregkh, stable, regressions
On Mon, 18 Aug 2025 20:31:02 +0800 wangzijie <wangzijie1@honor.com> wrote:
> To avoid potential UAF issues during module removal races, we use pde_set_flags()
> to save proc_ops flags in PDE itself before proc_register(), and then use
> pde_has_proc_*() helpers instead of directly dereferencing pde->proc_ops->*.
>
> However, the pde_set_flags() call was missing when creating net related proc files.
> This omission caused incorrect behavior which FMODE_LSEEK was being cleared
> inappropriately in proc_reg_open() for net proc files. Lars reported it in this link[1].
>
> Fix this by ensuring pde_set_flags() is called when register proc entry, and add
> NULL check for proc_ops in pde_set_flags().
>
> [1]: https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
>
> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al)
> Cc: stable@vger.kernel.org
> Reported-by: Lars Wendler <polynomial-c@gmx.de>
> Signed-off-by: wangzijie <wangzijie1@honor.com>
Could someone(s) please review this?
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 76e800e38..003031839 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -367,6 +367,23 @@ static const struct inode_operations proc_dir_inode_operations = {
> .setattr = proc_notify_change,
> };
>
> +static void pde_set_flags(struct proc_dir_entry *pde)
> +{
> + if (!pde->proc_ops)
> + return;
> +
> + if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
> + pde->flags |= PROC_ENTRY_PERMANENT;
> + if (pde->proc_ops->proc_read_iter)
> + pde->flags |= PROC_ENTRY_proc_read_iter;
> +#ifdef CONFIG_COMPAT
> + if (pde->proc_ops->proc_compat_ioctl)
> + pde->flags |= PROC_ENTRY_proc_compat_ioctl;
> +#endif
> + if (pde->proc_ops->proc_lseek)
> + pde->flags |= PROC_ENTRY_proc_lseek;
> +}
> +
> /* returns the registered entry, or frees dp and returns NULL on failure */
> struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
> struct proc_dir_entry *dp)
> @@ -374,6 +391,8 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
> if (proc_alloc_inum(&dp->low_ino))
> goto out_free_entry;
>
> + pde_set_flags(dp);
> +
> write_lock(&proc_subdir_lock);
> dp->parent = dir;
> if (pde_subdir_insert(dir, dp) == false) {
> @@ -561,20 +580,6 @@ 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)
> -{
> - if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
> - pde->flags |= PROC_ENTRY_PERMANENT;
> - if (pde->proc_ops->proc_read_iter)
> - pde->flags |= PROC_ENTRY_proc_read_iter;
> -#ifdef CONFIG_COMPAT
> - if (pde->proc_ops->proc_compat_ioctl)
> - pde->flags |= PROC_ENTRY_proc_compat_ioctl;
> -#endif
> - if (pde->proc_ops->proc_lseek)
> - pde->flags |= PROC_ENTRY_proc_lseek;
> -}
> -
> struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
> struct proc_dir_entry *parent,
> const struct proc_ops *proc_ops, void *data)
> @@ -585,7 +590,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
> if (!p)
> return NULL;
> p->proc_ops = proc_ops;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_data);
> @@ -636,7 +640,6 @@ struct proc_dir_entry *proc_create_seq_private(const char *name, umode_t mode,
> p->proc_ops = &proc_seq_ops;
> p->seq_ops = ops;
> p->state_size = state_size;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_seq_private);
> @@ -667,7 +670,6 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
> return NULL;
> p->proc_ops = &proc_single_ops;
> p->single_show = show;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_single_data);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files
2025-08-18 12:31 [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files wangzijie
2025-08-21 4:28 ` Andrew Morton
@ 2025-08-21 7:35 ` Christian Brauner
2025-08-21 8:12 ` wangzijie
1 sibling, 1 reply; 4+ messages in thread
From: Christian Brauner @ 2025-08-21 7:35 UTC (permalink / raw)
To: wangzijie
Cc: akpm, viro, adobriyan, rick.p.edgecombe, ast, k.shutemov,
jirislaby, linux-fsdevel, polynomial-c, gregkh, stable,
regressions
On Mon, Aug 18, 2025 at 08:31:02PM +0800, wangzijie wrote:
> To avoid potential UAF issues during module removal races, we use pde_set_flags()
> to save proc_ops flags in PDE itself before proc_register(), and then use
> pde_has_proc_*() helpers instead of directly dereferencing pde->proc_ops->*.
>
> However, the pde_set_flags() call was missing when creating net related proc files.
> This omission caused incorrect behavior which FMODE_LSEEK was being cleared
> inappropriately in proc_reg_open() for net proc files. Lars reported it in this link[1].
>
> Fix this by ensuring pde_set_flags() is called when register proc entry, and add
> NULL check for proc_ops in pde_set_flags().
>
> [1]: https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
>
> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al)
> Cc: stable@vger.kernel.org
> Reported-by: Lars Wendler <polynomial-c@gmx.de>
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
> v2:
> - followed by Jiri's suggestion to refractor code and reformat commit message
> ---
> fs/proc/generic.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 76e800e38..003031839 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -367,6 +367,23 @@ static const struct inode_operations proc_dir_inode_operations = {
> .setattr = proc_notify_change,
> };
>
> +static void pde_set_flags(struct proc_dir_entry *pde)
> +{
Stash pde->proc_ops in a local const variable instead of chasing the
pointer multiple times. Aside from that also makes it easier to read.
Otherwise seems fine.
> + if (!pde->proc_ops)
> + return;
> +
> + if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
> + pde->flags |= PROC_ENTRY_PERMANENT;
> + if (pde->proc_ops->proc_read_iter)
> + pde->flags |= PROC_ENTRY_proc_read_iter;
> +#ifdef CONFIG_COMPAT
> + if (pde->proc_ops->proc_compat_ioctl)
> + pde->flags |= PROC_ENTRY_proc_compat_ioctl;
> +#endif
> + if (pde->proc_ops->proc_lseek)
> + pde->flags |= PROC_ENTRY_proc_lseek;
> +}
> +
> /* returns the registered entry, or frees dp and returns NULL on failure */
> struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
> struct proc_dir_entry *dp)
> @@ -374,6 +391,8 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
> if (proc_alloc_inum(&dp->low_ino))
> goto out_free_entry;
>
> + pde_set_flags(dp);
> +
> write_lock(&proc_subdir_lock);
> dp->parent = dir;
> if (pde_subdir_insert(dir, dp) == false) {
> @@ -561,20 +580,6 @@ 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)
> -{
> - if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
> - pde->flags |= PROC_ENTRY_PERMANENT;
> - if (pde->proc_ops->proc_read_iter)
> - pde->flags |= PROC_ENTRY_proc_read_iter;
> -#ifdef CONFIG_COMPAT
> - if (pde->proc_ops->proc_compat_ioctl)
> - pde->flags |= PROC_ENTRY_proc_compat_ioctl;
> -#endif
> - if (pde->proc_ops->proc_lseek)
> - pde->flags |= PROC_ENTRY_proc_lseek;
> -}
> -
> struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
> struct proc_dir_entry *parent,
> const struct proc_ops *proc_ops, void *data)
> @@ -585,7 +590,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
> if (!p)
> return NULL;
> p->proc_ops = proc_ops;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_data);
> @@ -636,7 +640,6 @@ struct proc_dir_entry *proc_create_seq_private(const char *name, umode_t mode,
> p->proc_ops = &proc_seq_ops;
> p->seq_ops = ops;
> p->state_size = state_size;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_seq_private);
> @@ -667,7 +670,6 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
> return NULL;
> p->proc_ops = &proc_single_ops;
> p->single_show = show;
> - pde_set_flags(p);
> return proc_register(parent, p);
> }
> EXPORT_SYMBOL(proc_create_single_data);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files
2025-08-21 7:35 ` Christian Brauner
@ 2025-08-21 8:12 ` wangzijie
0 siblings, 0 replies; 4+ messages in thread
From: wangzijie @ 2025-08-21 8:12 UTC (permalink / raw)
To: brauner
Cc: adobriyan, akpm, ast, gregkh, jirislaby, k.shutemov,
linux-fsdevel, polynomial-c, regressions, rick.p.edgecombe,
stable, viro, wangzijie1
>On Mon, Aug 18, 2025 at 08:31:02PM +0800, wangzijie wrote:
>> To avoid potential UAF issues during module removal races, we use pde_set_flags()
>> to save proc_ops flags in PDE itself before proc_register(), and then use
>> pde_has_proc_*() helpers instead of directly dereferencing pde->proc_ops->*.
>>
>> However, the pde_set_flags() call was missing when creating net related proc files.
>> This omission caused incorrect behavior which FMODE_LSEEK was being cleared
>> inappropriately in proc_reg_open() for net proc files. Lars reported it in this link[1].
>>
>> Fix this by ensuring pde_set_flags() is called when register proc entry, and add
>> NULL check for proc_ops in pde_set_flags().
>>
>> [1]: https://lore.kernel.org/all/20250815195616.64497967@chagall.paradoxon.rec/
>>
>> Fixes: ff7ec8dc1b64 ("proc: use the same treatment to check proc_lseek as ones for proc_read_iter et.al)
>> Cc: stable@vger.kernel.org
>> Reported-by: Lars Wendler <polynomial-c@gmx.de>
>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>> ---
>> v2:
>> - followed by Jiri's suggestion to refractor code and reformat commit message
>> ---
>> fs/proc/generic.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
>> index 76e800e38..003031839 100644
>> --- a/fs/proc/generic.c
>> +++ b/fs/proc/generic.c
>> @@ -367,6 +367,23 @@ static const struct inode_operations proc_dir_inode_operations = {
>> .setattr = proc_notify_change,
>> };
>>
>> +static void pde_set_flags(struct proc_dir_entry *pde)
>> +{
>
>Stash pde->proc_ops in a local const variable instead of chasing the
>pointer multiple times. Aside from that also makes it easier to read.
>Otherwise seems fine.
Thanks for helping review, Christian. I will follow your suggestion.
>> + if (!pde->proc_ops)
>> + return;
>> +
>> + if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
>> + pde->flags |= PROC_ENTRY_PERMANENT;
>> + if (pde->proc_ops->proc_read_iter)
>> + pde->flags |= PROC_ENTRY_proc_read_iter;
>> +#ifdef CONFIG_COMPAT
>> + if (pde->proc_ops->proc_compat_ioctl)
>> + pde->flags |= PROC_ENTRY_proc_compat_ioctl;
>> +#endif
>> + if (pde->proc_ops->proc_lseek)
>> + pde->flags |= PROC_ENTRY_proc_lseek;
>> +}
>> +
>> /* returns the registered entry, or frees dp and returns NULL on failure */
>> struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
>> struct proc_dir_entry *dp)
>> @@ -374,6 +391,8 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
>> if (proc_alloc_inum(&dp->low_ino))
>> goto out_free_entry;
>>
>> + pde_set_flags(dp);
>> +
>> write_lock(&proc_subdir_lock);
>> dp->parent = dir;
>> if (pde_subdir_insert(dir, dp) == false) {
>> @@ -561,20 +580,6 @@ 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)
>> -{
>> - if (pde->proc_ops->proc_flags & PROC_ENTRY_PERMANENT)
>> - pde->flags |= PROC_ENTRY_PERMANENT;
>> - if (pde->proc_ops->proc_read_iter)
>> - pde->flags |= PROC_ENTRY_proc_read_iter;
>> -#ifdef CONFIG_COMPAT
>> - if (pde->proc_ops->proc_compat_ioctl)
>> - pde->flags |= PROC_ENTRY_proc_compat_ioctl;
>> -#endif
>> - if (pde->proc_ops->proc_lseek)
>> - pde->flags |= PROC_ENTRY_proc_lseek;
>> -}
>> -
>> struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
>> struct proc_dir_entry *parent,
>> const struct proc_ops *proc_ops, void *data)
>> @@ -585,7 +590,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode,
>> if (!p)
>> return NULL;
>> p->proc_ops = proc_ops;
>> - pde_set_flags(p);
>> return proc_register(parent, p);
>> }
>> EXPORT_SYMBOL(proc_create_data);
>> @@ -636,7 +640,6 @@ struct proc_dir_entry *proc_create_seq_private(const char *name, umode_t mode,
>> p->proc_ops = &proc_seq_ops;
>> p->seq_ops = ops;
>> p->state_size = state_size;
>> - pde_set_flags(p);
>> return proc_register(parent, p);
>> }
>> EXPORT_SYMBOL(proc_create_seq_private);
>> @@ -667,7 +670,6 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
>> return NULL;
>> p->proc_ops = &proc_single_ops;
>> p->single_show = show;
>> - pde_set_flags(p);
>> return proc_register(parent, p);
>> }
>> EXPORT_SYMBOL(proc_create_single_data);
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-21 8:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 12:31 [PATCH RESEND v2] proc: fix missing pde_set_flags() for net proc files wangzijie
2025-08-21 4:28 ` Andrew Morton
2025-08-21 7:35 ` Christian Brauner
2025-08-21 8:12 ` wangzijie
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).