* [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
@ 2025-08-18 4:05 wangzijie
2025-08-18 5:01 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: wangzijie @ 2025-08-18 4:05 UTC (permalink / raw)
To: akpm, viro, adobriyan, rick.p.edgecombe, ast, kirill.shutemov
Cc: polynomial-c, gregkh, stable, regressions, wangzijie
For avoiding pde->proc_ops->... dereference(which may cause UAF in rmmod race scene),
we call pde_set_flags() to save this kind of information in PDE itself before
proc_register() and call pde_has_proc_XXX() to replace pde->proc_ops->... dereference.
But there has omission of pde_set_flags() in net related proc file create, which cause
the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for net related proc file
after commit ff7ec8dc1b64("proc: use the same treatment to check proc_lseek as ones for
proc_read_iter et.al"). Lars reported it in this link[1]. So call pde_set_flags() when
create net related proc file to fix this bug.
[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)
Signed-off-by: wangzijie <wangzijie1@honor.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 76e800e38..57ec5e385 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 e737401d7..c4f7cbc7d 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 *);
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 52f0b75cb..20bc7481b 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.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
2025-08-18 4:05 [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file wangzijie
@ 2025-08-18 5:01 ` Jiri Slaby
2025-08-18 8:01 ` wangzijie
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2025-08-18 5:01 UTC (permalink / raw)
To: wangzijie, akpm, viro, adobriyan, rick.p.edgecombe, ast,
kirill.shutemov
Cc: polynomial-c, gregkh, stable, regressions
Hi,
On 18. 08. 25, 6:05, wangzijie wrote:
> For avoiding pde->proc_ops->... dereference(which may cause UAF in rmmod race scene),
> we call pde_set_flags() to save this kind of information in PDE itself before
> proc_register() and call pde_has_proc_XXX() to replace pde->proc_ops->... dereference.
> But there has omission of pde_set_flags() in net related proc file create, which cause
> the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for net related proc file
> after commit ff7ec8dc1b64("proc: use the same treatment to check proc_lseek as ones for
> proc_read_iter et.al"). Lars reported it in this link[1]. So call pde_set_flags() when
> create net related proc file to fix this bug.
I wonder, why is pde_set_flags() not a part of proc_register()?
Could you also use some LLM to reformat the message into something
comprehensible?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
2025-08-18 5:01 ` Jiri Slaby
@ 2025-08-18 8:01 ` wangzijie
2025-08-18 8:08 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: wangzijie @ 2025-08-18 8:01 UTC (permalink / raw)
To: jirislaby
Cc: adobriyan, akpm, ast, gregkh, kirill.shutemov, polynomial-c,
regressions, rick.p.edgecombe, stable, viro, wangzijie1
>Hi,
>
>On 18. 08. 25, 6:05, wangzijie wrote:
>> For avoiding pde->proc_ops->... dereference(which may cause UAF in rmmod race scene),
>> we call pde_set_flags() to save this kind of information in PDE itself before
>> proc_register() and call pde_has_proc_XXX() to replace pde->proc_ops->... dereference.
>> But there has omission of pde_set_flags() in net related proc file create, which cause
>> the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for net related proc file
>> after commit ff7ec8dc1b64("proc: use the same treatment to check proc_lseek as ones for
>> proc_read_iter et.al"). Lars reported it in this link[1]. So call pde_set_flags() when
>> create net related proc file to fix this bug.
>
>I wonder, why is pde_set_flags() not a part of proc_register()?
Not all proc_dir_entry have proc_ops, so you mean this will be a better modification?
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 76e800e38..d52197c35 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -367,6 +367,20 @@ 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->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 +388,9 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
if (proc_alloc_inum(&dp->low_ino))
goto out_free_entry;
+ if (dp->proc_ops)
+ pde_set_flags(dp);
+
write_lock(&proc_subdir_lock);
dp->parent = dir;
if (pde_subdir_insert(dir, dp) == false) {
>Could you also use some LLM to reformat the message into something
>comprehensible?
English is not my native language, so using LLM to format message is
a good suggestion for me, Thank you.
>thanks,
>--
>js
>suse labs
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
2025-08-18 8:01 ` wangzijie
@ 2025-08-18 8:08 ` Jiri Slaby
2025-08-18 8:11 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2025-08-18 8:08 UTC (permalink / raw)
To: wangzijie, jirislaby
Cc: adobriyan, akpm, ast, gregkh, kirill.shutemov, polynomial-c,
regressions, rick.p.edgecombe, stable, viro
On 18. 08. 25, 10:01, wangzijie wrote:
>> Hi,
>>
>> On 18. 08. 25, 6:05, wangzijie wrote:
>>> For avoiding pde->proc_ops->... dereference(which may cause UAF in rmmod race scene),
>>> we call pde_set_flags() to save this kind of information in PDE itself before
>>> proc_register() and call pde_has_proc_XXX() to replace pde->proc_ops->... dereference.
>>> But there has omission of pde_set_flags() in net related proc file create, which cause
>>> the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for net related proc file
>>> after commit ff7ec8dc1b64("proc: use the same treatment to check proc_lseek as ones for
>>> proc_read_iter et.al"). Lars reported it in this link[1]. So call pde_set_flags() when
>>> create net related proc file to fix this bug.
>>
>> I wonder, why is pde_set_flags() not a part of proc_register()?
>
> Not all proc_dir_entry have proc_ops, so you mean this will be a better modification?
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 76e800e38..d52197c35 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -367,6 +367,20 @@ 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->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 +388,9 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,
> if (proc_alloc_inum(&dp->low_ino))
> goto out_free_entry;
>
> + if (dp->proc_ops)
> + pde_set_flags(dp);
This occurs much saner and better to me. But I am no FS fellow. It's up
to them to tell if it is the right thing to do.
(The above hunks are missing removal of pde_set_flags() from the
original location and removal of now unnecessary calls of
pde_set_flags(). But I assume you would do that in a real/submitted patch.)
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
2025-08-18 8:08 ` Jiri Slaby
@ 2025-08-18 8:11 ` Jiri Slaby
2025-08-18 8:29 ` wangzijie
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2025-08-18 8:11 UTC (permalink / raw)
To: wangzijie
Cc: adobriyan, akpm, ast, gregkh, polynomial-c, regressions,
rick.p.edgecombe, stable, viro, k.shutemov
k.shutemov no longer @ intel -> use gmail.
On 18. 08. 25, 10:08, Jiri Slaby wrote:
> On 18. 08. 25, 10:01, wangzijie wrote:
>>> Hi,
>>>
>>> On 18. 08. 25, 6:05, wangzijie wrote:
>>>> For avoiding pde->proc_ops->... dereference(which may cause UAF in
>>>> rmmod race scene),
>>>> we call pde_set_flags() to save this kind of information in PDE
>>>> itself before
>>>> proc_register() and call pde_has_proc_XXX() to replace pde-
>>>> >proc_ops->... dereference.
>>>> But there has omission of pde_set_flags() in net related proc file
>>>> create, which cause
>>>> the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for
>>>> net related proc file
>>>> after commit ff7ec8dc1b64("proc: use the same treatment to check
>>>> proc_lseek as ones for
>>>> proc_read_iter et.al"). Lars reported it in this link[1]. So call
>>>> pde_set_flags() when
>>>> create net related proc file to fix this bug.
>>>
>>> I wonder, why is pde_set_flags() not a part of proc_register()?
>>
>> Not all proc_dir_entry have proc_ops, so you mean this will be a
>> better modification?
>>
>> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
>> index 76e800e38..d52197c35 100644
>> --- a/fs/proc/generic.c
>> +++ b/fs/proc/generic.c
>> @@ -367,6 +367,20 @@ 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->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 +388,9 @@ struct proc_dir_entry *proc_register(struct
>> proc_dir_entry *dir,
>> if (proc_alloc_inum(&dp->low_ino))
>> goto out_free_entry;
>>
>> + if (dp->proc_ops)
And to be honest, I would possibly move the 'if' into pde_set_flags().
>> + pde_set_flags(dp);
As here, it is not completely clear why you would want to test
"dp->proc_ops" for something called "pde_set_flags".
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file
2025-08-18 8:11 ` Jiri Slaby
@ 2025-08-18 8:29 ` wangzijie
0 siblings, 0 replies; 6+ messages in thread
From: wangzijie @ 2025-08-18 8:29 UTC (permalink / raw)
To: jirislaby
Cc: adobriyan, akpm, ast, gregkh, k.shutemov, polynomial-c,
regressions, rick.p.edgecombe, stable, viro, wangzijie1
>k.shutemov no longer @ intel -> use gmail.
>
>On 18. 08. 25, 10:08, Jiri Slaby wrote:
>> On 18. 08. 25, 10:01, wangzijie wrote:
>>>> Hi,
>>>>
>>>> On 18. 08. 25, 6:05, wangzijie wrote:
>>>>> For avoiding pde->proc_ops->... dereference(which may cause UAF in
>>>>> rmmod race scene),
>>>>> we call pde_set_flags() to save this kind of information in PDE
>>>>> itself before
>>>>> proc_register() and call pde_has_proc_XXX() to replace pde-
>>>>> >proc_ops->... dereference.
>>>>> But there has omission of pde_set_flags() in net related proc file
>>>>> create, which cause
>>>>> the wroing behavior of FMODE_LSEEK clearing in proc_reg_open() for
>>>>> net related proc file
>>>>> after commit ff7ec8dc1b64("proc: use the same treatment to check
>>>>> proc_lseek as ones for
>>>>> proc_read_iter et.al"). Lars reported it in this link[1]. So call
>>>>> pde_set_flags() when
>>>>> create net related proc file to fix this bug.
>>>>
>>>> I wonder, why is pde_set_flags() not a part of proc_register()?
>>>
>>> Not all proc_dir_entry have proc_ops, so you mean this will be a
>>> better modification?
>>>
>>> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
>>> index 76e800e38..d52197c35 100644
>>> --- a/fs/proc/generic.c
>>> +++ b/fs/proc/generic.c
>>> @@ -367,6 +367,20 @@ 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->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 +388,9 @@ struct proc_dir_entry *proc_register(struct
>>> proc_dir_entry *dir,
>>> if (proc_alloc_inum(&dp->low_ino))
>>> goto out_free_entry;
>>>
>>> + if (dp->proc_ops)
>
>And to be honest, I would possibly move the 'if' into pde_set_flags().
>
>>> + pde_set_flags(dp);
>
>As here, it is not completely clear why you would want to test
>"dp->proc_ops" for something called "pde_set_flags".
>
>--
>js
>suse labs
Thank you for all suggestions, it makes sense to me. Let me submit the
next version patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-18 8:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 4:05 [PATCH] proc: fix wrong behavior of FMODE_LSEEK clearing for net related proc file wangzijie
2025-08-18 5:01 ` Jiri Slaby
2025-08-18 8:01 ` wangzijie
2025-08-18 8:08 ` Jiri Slaby
2025-08-18 8:11 ` Jiri Slaby
2025-08-18 8:29 ` wangzijie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox