* [PATCH] sysfs: check visibility before changing group attribute ownership
@ 2025-10-16 10:14 Fernando Fernandez Mancera
2025-10-16 14:46 ` Greg KH
2025-10-16 15:38 ` Jakub Kicinski
0 siblings, 2 replies; 6+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 10:14 UTC (permalink / raw)
To: netdev, linux-kernel, gregkh, kuba
Cc: cynthia, rafael, dakr, christian.brauner, edumazet, pabeni, davem,
horms, Fernando Fernandez Mancera
Since commit 0c17270f9b92 ("net: sysfs: Implement is_visible for
phys_(port_id, port_name, switch_id)"), __dev_change_net_namespace() can
hit WARN_ON() when trying to change owner of a file that isn't visible.
See the trace below:
WARNING: CPU: 6 PID: 2938 at net/core/dev.c:12410 __dev_change_net_namespace+0xb89/0xc30
CPU: 6 UID: 0 PID: 2938 Comm: incusd Not tainted 6.17.1-1-mainline #1 PREEMPT(full) 4b783b4a638669fb644857f484487d17cb45ed1f
Hardware name: Framework Laptop 13 (AMD Ryzen 7040Series)/FRANMDCP07, BIOS 03.07 02/19/2025
RIP: 0010:__dev_change_net_namespace+0xb89/0xc30
[...]
Call Trace:
<TASK>
? if6_seq_show+0x30/0x50
do_setlink.isra.0+0xc7/0x1270
? __nla_validate_parse+0x5c/0xcc0
? security_capable+0x94/0x1a0
rtnl_newlink+0x858/0xc20
? update_curr+0x8e/0x1c0
? update_entity_lag+0x71/0x80
? sched_balance_newidle+0x358/0x450
? psi_task_switch+0x113/0x2a0
? __pfx_rtnl_newlink+0x10/0x10
rtnetlink_rcv_msg+0x346/0x3e0
? sched_clock+0x10/0x30
? __pfx_rtnetlink_rcv_msg+0x10/0x10
netlink_rcv_skb+0x59/0x110
netlink_unicast+0x285/0x3c0
? __alloc_skb+0xdb/0x1a0
netlink_sendmsg+0x20d/0x430
____sys_sendmsg+0x39f/0x3d0
? import_iovec+0x2f/0x40
___sys_sendmsg+0x99/0xe0
__sys_sendmsg+0x8a/0xf0
do_syscall_64+0x81/0x970
? __sys_bind+0xe3/0x110
? syscall_exit_work+0x143/0x1b0
? do_syscall_64+0x244/0x970
? sock_alloc_file+0x63/0xc0
? syscall_exit_work+0x143/0x1b0
? do_syscall_64+0x244/0x970
? alloc_fd+0x12e/0x190
? put_unused_fd+0x2a/0x70
? do_sys_openat2+0xa2/0xe0
? syscall_exit_work+0x143/0x1b0
? do_syscall_64+0x244/0x970
? exc_page_fault+0x7e/0x1a0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
[...]
</TASK>
Fix this by checking is_visible() before trying to touch the attribute.
Fixes: 303a42769c4c ("sysfs: add sysfs_group{s}_change_owner()")
Reported-by: Cynthia <cynthia@kosmx.dev>
Closes: https://lore.kernel.org/netdev/01070199e22de7f8-28f711ab-d3f1-46d9-b9a0-048ab05eb09b-000000@eu-central-1.amazonses.com/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
fs/sysfs/group.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 2d78e94072a0..e142bac4f9f8 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -498,17 +498,26 @@ int compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj,
}
EXPORT_SYMBOL_GPL(compat_only_sysfs_link_entry_to_kobj);
-static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn,
+static int sysfs_group_attrs_change_owner(struct kobject *kobj,
+ struct kernfs_node *grp_kn,
const struct attribute_group *grp,
struct iattr *newattrs)
{
struct kernfs_node *kn;
- int error;
+ int error, i;
+ umode_t mode;
if (grp->attrs) {
struct attribute *const *attr;
- for (attr = grp->attrs; *attr; attr++) {
+ for (i = 0, attr = grp->attrs; *attr; i++, attr++) {
+ if (grp->is_visible) {
+ mode = grp->is_visible(kobj, *attr, i);
+ if (mode & SYSFS_GROUP_INVISIBLE)
+ break;
+ if (!mode)
+ continue;
+ }
kn = kernfs_find_and_get(grp_kn, (*attr)->name);
if (!kn)
return -ENOENT;
@@ -523,7 +532,14 @@ static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn,
if (grp->bin_attrs) {
const struct bin_attribute *const *bin_attr;
- for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
+ for (i = 0, bin_attr = grp->bin_attrs; *bin_attr; i++, bin_attr++) {
+ if (grp->is_bin_visible) {
+ mode = grp->is_bin_visible(kobj, *bin_attr, i);
+ if (mode & SYSFS_GROUP_INVISIBLE)
+ break;
+ if (!mode)
+ continue;
+ }
kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name);
if (!kn)
return -ENOENT;
@@ -573,7 +589,7 @@ int sysfs_group_change_owner(struct kobject *kobj,
error = kernfs_setattr(grp_kn, &newattrs);
if (!error)
- error = sysfs_group_attrs_change_owner(grp_kn, grp, &newattrs);
+ error = sysfs_group_attrs_change_owner(kobj, grp_kn, grp, &newattrs);
kernfs_put(grp_kn);
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sysfs: check visibility before changing group attribute ownership
2025-10-16 10:14 [PATCH] sysfs: check visibility before changing group attribute ownership Fernando Fernandez Mancera
@ 2025-10-16 14:46 ` Greg KH
2025-10-16 15:31 ` Cynthia
2025-10-16 15:38 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2025-10-16 14:46 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, linux-kernel, kuba, cynthia, rafael, dakr,
christian.brauner, edumazet, pabeni, davem, horms
On Thu, Oct 16, 2025 at 12:14:56PM +0200, Fernando Fernandez Mancera wrote:
> Since commit 0c17270f9b92 ("net: sysfs: Implement is_visible for
> phys_(port_id, port_name, switch_id)"), __dev_change_net_namespace() can
> hit WARN_ON() when trying to change owner of a file that isn't visible.
> See the trace below:
>
> WARNING: CPU: 6 PID: 2938 at net/core/dev.c:12410 __dev_change_net_namespace+0xb89/0xc30
> CPU: 6 UID: 0 PID: 2938 Comm: incusd Not tainted 6.17.1-1-mainline #1 PREEMPT(full) 4b783b4a638669fb644857f484487d17cb45ed1f
> Hardware name: Framework Laptop 13 (AMD Ryzen 7040Series)/FRANMDCP07, BIOS 03.07 02/19/2025
> RIP: 0010:__dev_change_net_namespace+0xb89/0xc30
> [...]
> Call Trace:
> <TASK>
> ? if6_seq_show+0x30/0x50
> do_setlink.isra.0+0xc7/0x1270
> ? __nla_validate_parse+0x5c/0xcc0
> ? security_capable+0x94/0x1a0
> rtnl_newlink+0x858/0xc20
> ? update_curr+0x8e/0x1c0
> ? update_entity_lag+0x71/0x80
> ? sched_balance_newidle+0x358/0x450
> ? psi_task_switch+0x113/0x2a0
> ? __pfx_rtnl_newlink+0x10/0x10
> rtnetlink_rcv_msg+0x346/0x3e0
> ? sched_clock+0x10/0x30
> ? __pfx_rtnetlink_rcv_msg+0x10/0x10
> netlink_rcv_skb+0x59/0x110
> netlink_unicast+0x285/0x3c0
> ? __alloc_skb+0xdb/0x1a0
> netlink_sendmsg+0x20d/0x430
> ____sys_sendmsg+0x39f/0x3d0
> ? import_iovec+0x2f/0x40
> ___sys_sendmsg+0x99/0xe0
> __sys_sendmsg+0x8a/0xf0
> do_syscall_64+0x81/0x970
> ? __sys_bind+0xe3/0x110
> ? syscall_exit_work+0x143/0x1b0
> ? do_syscall_64+0x244/0x970
> ? sock_alloc_file+0x63/0xc0
> ? syscall_exit_work+0x143/0x1b0
> ? do_syscall_64+0x244/0x970
> ? alloc_fd+0x12e/0x190
> ? put_unused_fd+0x2a/0x70
> ? do_sys_openat2+0xa2/0xe0
> ? syscall_exit_work+0x143/0x1b0
> ? do_syscall_64+0x244/0x970
> ? exc_page_fault+0x7e/0x1a0
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [...]
> </TASK>
>
> Fix this by checking is_visible() before trying to touch the attribute.
>
> Fixes: 303a42769c4c ("sysfs: add sysfs_group{s}_change_owner()")
> Reported-by: Cynthia <cynthia@kosmx.dev>
> Closes: https://lore.kernel.org/netdev/01070199e22de7f8-28f711ab-d3f1-46d9-b9a0-048ab05eb09b-000000@eu-central-1.amazonses.com/
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> ---
> fs/sysfs/group.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
Nice, thanks! This has been tested, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sysfs: check visibility before changing group attribute ownership
2025-10-16 14:46 ` Greg KH
@ 2025-10-16 15:31 ` Cynthia
2025-10-16 23:17 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 6+ messages in thread
From: Cynthia @ 2025-10-16 15:31 UTC (permalink / raw)
To: Greg KH, Fernando Fernandez Mancera
Cc: netdev, linux-kernel, kuba, rafael, dakr, christian.brauner,
edumazet, pabeni, davem, horms
On 10/16/25 16:46, Greg KH wrote:
> On Thu, Oct 16, 2025 at 12:14:56PM +0200, Fernando Fernandez Mancera wrote:
>> Since commit 0c17270f9b92 ("net: sysfs: Implement is_visible for
>> phys_(port_id, port_name, switch_id)"), __dev_change_net_namespace() can
>> hit WARN_ON() when trying to change owner of a file that isn't visible.
>> See the trace below:
>>
>> WARNING: CPU: 6 PID: 2938 at net/core/dev.c:12410 __dev_change_net_namespace+0xb89/0xc30
>> CPU: 6 UID: 0 PID: 2938 Comm: incusd Not tainted 6.17.1-1-mainline #1 PREEMPT(full) 4b783b4a638669fb644857f484487d17cb45ed1f
>> Hardware name: Framework Laptop 13 (AMD Ryzen 7040Series)/FRANMDCP07, BIOS 03.07 02/19/2025
>> RIP: 0010:__dev_change_net_namespace+0xb89/0xc30
>> [...]
>> Call Trace:
>> <TASK>
>> ? if6_seq_show+0x30/0x50
>> do_setlink.isra.0+0xc7/0x1270
>> ? __nla_validate_parse+0x5c/0xcc0
>> ? security_capable+0x94/0x1a0
>> rtnl_newlink+0x858/0xc20
>> ? update_curr+0x8e/0x1c0
>> ? update_entity_lag+0x71/0x80
>> ? sched_balance_newidle+0x358/0x450
>> ? psi_task_switch+0x113/0x2a0
>> ? __pfx_rtnl_newlink+0x10/0x10
>> rtnetlink_rcv_msg+0x346/0x3e0
>> ? sched_clock+0x10/0x30
>> ? __pfx_rtnetlink_rcv_msg+0x10/0x10
>> netlink_rcv_skb+0x59/0x110
>> netlink_unicast+0x285/0x3c0
>> ? __alloc_skb+0xdb/0x1a0
>> netlink_sendmsg+0x20d/0x430
>> ____sys_sendmsg+0x39f/0x3d0
>> ? import_iovec+0x2f/0x40
>> ___sys_sendmsg+0x99/0xe0
>> __sys_sendmsg+0x8a/0xf0
>> do_syscall_64+0x81/0x970
>> ? __sys_bind+0xe3/0x110
>> ? syscall_exit_work+0x143/0x1b0
>> ? do_syscall_64+0x244/0x970
>> ? sock_alloc_file+0x63/0xc0
>> ? syscall_exit_work+0x143/0x1b0
>> ? do_syscall_64+0x244/0x970
>> ? alloc_fd+0x12e/0x190
>> ? put_unused_fd+0x2a/0x70
>> ? do_sys_openat2+0xa2/0xe0
>> ? syscall_exit_work+0x143/0x1b0
>> ? do_syscall_64+0x244/0x970
>> ? exc_page_fault+0x7e/0x1a0
>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [...]
>> </TASK>
>>
>> Fix this by checking is_visible() before trying to touch the attribute.
>>
>> Fixes: 303a42769c4c ("sysfs: add sysfs_group{s}_change_owner()")
>> Reported-by: Cynthia <cynthia@kosmx.dev>
>> Closes: https://lore.kernel.org/netdev/01070199e22de7f8-28f711ab-d3f1-46d9-b9a0-048ab05eb09b-000000@eu-central-1.amazonses.com/
>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>> ---
>> fs/sysfs/group.c | 26 +++++++++++++++++++++-----
>> 1 file changed, 21 insertions(+), 5 deletions(-)
> Nice, thanks! This has been tested, right?
>
> thanks,
>
> greg k-h
I did a quick test just now, it works in the VM (no warn and the
container is running).
kosmx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sysfs: check visibility before changing group attribute ownership
2025-10-16 10:14 [PATCH] sysfs: check visibility before changing group attribute ownership Fernando Fernandez Mancera
2025-10-16 14:46 ` Greg KH
@ 2025-10-16 15:38 ` Jakub Kicinski
2025-10-16 23:30 ` Fernando Fernandez Mancera
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-10-16 15:38 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, linux-kernel, gregkh, cynthia, rafael, dakr,
christian.brauner, edumazet, pabeni, davem, horms
On Thu, 16 Oct 2025 12:14:56 +0200 Fernando Fernandez Mancera wrote:
> Since commit 0c17270f9b92 ("net: sysfs: Implement is_visible for
> phys_(port_id, port_name, switch_id)"), __dev_change_net_namespace() can
> hit WARN_ON() when trying to change owner of a file that isn't visible.
> See the trace below:
Dunno much about sysfs but this is what I had in mind so FWIW:
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
I'd be tempted to chuck:
Fixes: 0c17270f9b92 ("net: sysfs: Implement is_visible for phys_(port_id, port_name, switch_id)")
here as well. Or are we certain there are other callers that could have
triggered this earlier?
> Reported-by: Cynthia <cynthia@kosmx.dev>
Perhaps:
Reported-and-bisected-by: ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sysfs: check visibility before changing group attribute ownership
2025-10-16 15:31 ` Cynthia
@ 2025-10-16 23:17 ` Fernando Fernandez Mancera
0 siblings, 0 replies; 6+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:17 UTC (permalink / raw)
To: Cynthia, Greg KH
Cc: netdev, linux-kernel, kuba, rafael, dakr, christian.brauner,
edumazet, pabeni, davem, horms
On 10/16/25 5:31 PM, Cynthia wrote:
> [...]
>>> ? psi_task_switch+0x113/0x2a0
>>> ? __pfx_rtnl_newlink+0x10/0x10
>>> rtnetlink_rcv_msg+0x346/0x3e0
>>> ? sched_clock+0x10/0x30
>>> ? __pfx_rtnetlink_rcv_msg+0x10/0x10
>>> netlink_rcv_skb+0x59/0x110
>>> netlink_unicast+0x285/0x3c0
>>> ? __alloc_skb+0xdb/0x1a0
>>> netlink_sendmsg+0x20d/0x430
>>> ____sys_sendmsg+0x39f/0x3d0
>>> ? import_iovec+0x2f/0x40
>>> ___sys_sendmsg+0x99/0xe0
>>> __sys_sendmsg+0x8a/0xf0
>>> do_syscall_64+0x81/0x970
>>> ? __sys_bind+0xe3/0x110
>>> ? syscall_exit_work+0x143/0x1b0
>>> ? do_syscall_64+0x244/0x970
>>> ? sock_alloc_file+0x63/0xc0
>>> ? syscall_exit_work+0x143/0x1b0
>>> ? do_syscall_64+0x244/0x970
>>> ? alloc_fd+0x12e/0x190
>>> ? put_unused_fd+0x2a/0x70
>>> ? do_sys_openat2+0xa2/0xe0
>>> ? syscall_exit_work+0x143/0x1b0
>>> ? do_syscall_64+0x244/0x970
>>> ? exc_page_fault+0x7e/0x1a0
>>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>> [...]
>>> </TASK>
>>>
>>> Fix this by checking is_visible() before trying to touch the attribute.
>>>
>>> Fixes: 303a42769c4c ("sysfs: add sysfs_group{s}_change_owner()")
>>> Reported-by: Cynthia <cynthia@kosmx.dev>
>>> Closes: https://lore.kernel.org/netdev/01070199e22de7f8-28f711ab-
>>> d3f1-46d9-b9a0-048ab05eb09b-000000@eu-central-1.amazonses.com/
>>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>>> ---
>>> fs/sysfs/group.c | 26 +++++++++++++++++++++-----
>>> 1 file changed, 21 insertions(+), 5 deletions(-)
>> Nice, thanks! This has been tested, right?
>>
>> thanks,
>>
>> greg k-h
>
> I did a quick test just now, it works in the VM (no warn and the
> container is running).
>
Same here, I tested it while doing the patch using the reproducer
provided on the report.
Thanks,
Fernando.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sysfs: check visibility before changing group attribute ownership
2025-10-16 15:38 ` Jakub Kicinski
@ 2025-10-16 23:30 ` Fernando Fernandez Mancera
0 siblings, 0 replies; 6+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:30 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, linux-kernel, gregkh, cynthia, rafael, dakr,
christian.brauner, edumazet, pabeni, davem, horms
On 10/16/25 5:38 PM, Jakub Kicinski wrote:
> On Thu, 16 Oct 2025 12:14:56 +0200 Fernando Fernandez Mancera wrote:
>> Since commit 0c17270f9b92 ("net: sysfs: Implement is_visible for
>> phys_(port_id, port_name, switch_id)"), __dev_change_net_namespace() can
>> hit WARN_ON() when trying to change owner of a file that isn't visible.
>> See the trace below:
>
> Dunno much about sysfs but this is what I had in mind so FWIW:
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
> I'd be tempted to chuck:
>
> Fixes: 0c17270f9b92 ("net: sysfs: Implement is_visible for phys_(port_id, port_name, switch_id)")
>
> here as well. Or are we certain there are other callers that could have
> triggered this earlier?
>
It is hard for me tell certainly. I am fine adding:
Fixes: 0c17270f9b92 ("net: sysfs: Implement is_visible for
phys_(port_id, port_name, switch_id)")
but I would keep the current Fixes tag too. IMHO, given that visibility
could return 0 for some attributes and > 0 for others in the same group,
is up to sysfs to check the attribute visibility before updating the
ownership of a whole group.. so this check should have been there since
the beginning.
Anyway, I am not an expert on sysfs neither..
>> Reported-by: Cynthia <cynthia@kosmx.dev>
>
> Perhaps:
>
> Reported-and-bisected-by: ...
>
Oh, sure, thanks! I am going to wait a bit to allow more feedback and
send a v2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-16 23:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 10:14 [PATCH] sysfs: check visibility before changing group attribute ownership Fernando Fernandez Mancera
2025-10-16 14:46 ` Greg KH
2025-10-16 15:31 ` Cynthia
2025-10-16 23:17 ` Fernando Fernandez Mancera
2025-10-16 15:38 ` Jakub Kicinski
2025-10-16 23:30 ` Fernando Fernandez Mancera
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).