* [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done
@ 2024-09-11 8:00 syzbot
2024-09-11 10:04 ` Maxime Chevallier
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
0 siblings, 2 replies; 8+ messages in thread
From: syzbot @ 2024-09-11 8:00 UTC (permalink / raw)
To: christophe.leroy, davem, edumazet, kuba, linux-kernel,
maxime.chevallier, netdev, pabeni, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: a9b1fab3b69f Merge branch 'ionic-convert-rx-queue-buffers-..
git tree: net-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1193c49f980000
kernel config: https://syzkaller.appspot.com/x/.config?x=37742f4fda0d1b09
dashboard link: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14bb7bc7980000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b0a100580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/0459f959b12d/disk-a9b1fab3.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/337f1be5353b/vmlinux-a9b1fab3.xz
kernel image: https://storage.googleapis.com/syzbot-assets/0e3701969c4a/bzImage-a9b1fab3.xz
The issue was bisected to:
commit 17194be4c8e1e82d8b484e58cdcb495c0714d1fd
Author: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Wed Aug 21 15:10:01 2024 +0000
net: ethtool: Introduce a command to list PHYs on an interface
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1034a49f980000
final oops: https://syzkaller.appspot.com/x/report.txt?x=1234a49f980000
console output: https://syzkaller.appspot.com/x/log.txt?x=1434a49f980000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
------------[ cut here ]------------
refcount_t: decrement hit 0; leaking memory.
WARNING: CPU: 0 PID: 5227 at lib/refcount.c:31 refcount_warn_saturate+0xfa/0x1d0 lib/refcount.c:31
Modules linked in:
CPU: 0 UID: 0 PID: 5227 Comm: syz-executor281 Not tainted 6.11.0-rc6-syzkaller-01235-ga9b1fab3b69f #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
RIP: 0010:refcount_warn_saturate+0xfa/0x1d0 lib/refcount.c:31
Code: b2 00 00 00 e8 e7 09 e1 fc 5b 5d c3 cc cc cc cc e8 db 09 e1 fc c6 05 56 d4 4f 0b 01 90 48 c7 c7 c0 3b 60 8c e8 f7 2d a3 fc 90 <0f> 0b 90 90 eb d9 e8 bb 09 e1 fc c6 05 33 d4 4f 0b 01 90 48 c7 c7
RSP: 0018:ffffc900033ff048 EFLAGS: 00010246
RAX: 033d767773c83300 RBX: ffff88802f8d0664 RCX: ffff8880506e0000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000004 R08: ffffffff8155b372 R09: 1ffff1101710519a
R10: dffffc0000000000 R11: ffffed101710519b R12: ffff88802f8d0620
R13: 0000000000000000 R14: ffff88802f8d0664 R15: dffffc0000000000
FS: 00005555884f4380(0000) GS:ffff8880b8800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055c9aad930a8 CR3: 0000000052acc000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__refcount_dec include/linux/refcount.h:336 [inline]
refcount_dec include/linux/refcount.h:351 [inline]
ref_tracker_free+0x6af/0x7e0 lib/ref_tracker.c:236
netdev_tracker_free include/linux/netdevice.h:4057 [inline]
netdev_put include/linux/netdevice.h:4074 [inline]
ethnl_parse_header_dev_put net/ethtool/netlink.h:271 [inline]
ethnl_phy_done+0x6f/0x100 net/ethtool/phy.c:238
genl_done+0x136/0x210 net/netlink/genetlink.c:1043
netlink_dump+0x9b2/0xd80 net/netlink/af_netlink.c:2370
__netlink_dump_start+0x5a2/0x790 net/netlink/af_netlink.c:2440
genl_family_rcv_msg_dumpit net/netlink/genetlink.c:1076 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1192 [inline]
genl_rcv_msg+0x88c/0xec0 net/netlink/genetlink.c:1210
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f7cf874a219
Code: 48 83 c4 28 c3 e8 e7 18 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffff97db218 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007ffff97db3e8 RCX: 00007f7cf874a219
RDX: 0000000000000000 RSI: 0000000020001900 RDI: 0000000000000003
RBP: 00007f7cf87bc610 R08: 00007ffff97db3e8 R09: 00007ffff97db3e8
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffff97db3d8 R14: 0000000000000001 R15: 0000000000000001
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done
2024-09-11 8:00 [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done syzbot
@ 2024-09-11 10:04 ` Maxime Chevallier
2024-09-11 10:08 ` Eric Dumazet
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
1 sibling, 1 reply; 8+ messages in thread
From: Maxime Chevallier @ 2024-09-11 10:04 UTC (permalink / raw)
To: syzbot
Cc: christophe.leroy, davem, edumazet, kuba, linux-kernel, netdev,
pabeni, syzkaller-bugs
Hi,
On Wed, 11 Sep 2024 01:00:23 -0700
syzbot <syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: a9b1fab3b69f Merge branch 'ionic-convert-rx-queue-buffers-..
> git tree: net-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=1193c49f980000
> kernel config: https://syzkaller.appspot.com/x/.config?x=37742f4fda0d1b09
> dashboard link: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14bb7bc7980000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b0a100580000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/0459f959b12d/disk-a9b1fab3.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/337f1be5353b/vmlinux-a9b1fab3.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/0e3701969c4a/bzImage-a9b1fab3.xz
>
> The issue was bisected to:
>
> commit 17194be4c8e1e82d8b484e58cdcb495c0714d1fd
> Author: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Date: Wed Aug 21 15:10:01 2024 +0000
>
> net: ethtool: Introduce a command to list PHYs on an interface
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1034a49f980000
> final oops: https://syzkaller.appspot.com/x/report.txt?x=1234a49f980000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1434a49f980000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
> Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
I'm currently investigating this. I couldn't reproduce it though, even
with the C reproducer, although this was on an arm64 box. I'll give it
a try on x86_64 with the provided .config, see if I can figure out
what's going on, as it looks like the ethnl_phy_start() doesn't get
called.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done
2024-09-11 10:04 ` Maxime Chevallier
@ 2024-09-11 10:08 ` Eric Dumazet
2024-09-11 11:40 ` Maxime Chevallier
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2024-09-11 10:08 UTC (permalink / raw)
To: Maxime Chevallier
Cc: syzbot, christophe.leroy, davem, kuba, linux-kernel, netdev,
pabeni, syzkaller-bugs
On Wed, Sep 11, 2024 at 12:04 PM Maxime Chevallier
<maxime.chevallier@bootlin.com> wrote:
>
> Hi,
>
> On Wed, 11 Sep 2024 01:00:23 -0700
> syzbot <syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com> wrote:
>
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: a9b1fab3b69f Merge branch 'ionic-convert-rx-queue-buffers-..
> > git tree: net-next
> > console+strace: https://syzkaller.appspot.com/x/log.txt?x=1193c49f980000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=37742f4fda0d1b09
> > dashboard link: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
> > compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14bb7bc7980000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17b0a100580000
> >
> > Downloadable assets:
> > disk image: https://storage.googleapis.com/syzbot-assets/0459f959b12d/disk-a9b1fab3.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/337f1be5353b/vmlinux-a9b1fab3.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/0e3701969c4a/bzImage-a9b1fab3.xz
> >
> > The issue was bisected to:
> >
> > commit 17194be4c8e1e82d8b484e58cdcb495c0714d1fd
> > Author: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > Date: Wed Aug 21 15:10:01 2024 +0000
> >
> > net: ethtool: Introduce a command to list PHYs on an interface
> >
> > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1034a49f980000
> > final oops: https://syzkaller.appspot.com/x/report.txt?x=1234a49f980000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1434a49f980000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
> > Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
>
> I'm currently investigating this. I couldn't reproduce it though, even
> with the C reproducer, although this was on an arm64 box. I'll give it
> a try on x86_64 with the provided .config, see if I can figure out
> what's going on, as it looks like the ethnl_phy_start() doesn't get
> called.
Make sure to have in your .config
CONFIG_REF_TRACKER=y
CONFIG_NET_DEV_REFCNT_TRACKER=y
CONFIG_NET_NS_REFCNT_TRACKER=y
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done
2024-09-11 10:08 ` Eric Dumazet
@ 2024-09-11 11:40 ` Maxime Chevallier
0 siblings, 0 replies; 8+ messages in thread
From: Maxime Chevallier @ 2024-09-11 11:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: syzbot, christophe.leroy, davem, kuba, linux-kernel, netdev,
pabeni, syzkaller-bugs
Hi Eric,
On Wed, 11 Sep 2024 12:08:36 +0200
Eric Dumazet <edumazet@google.com> wrote:
> > I'm currently investigating this. I couldn't reproduce it though, even
> > with the C reproducer, although this was on an arm64 box. I'll give it
> > a try on x86_64 with the provided .config, see if I can figure out
> > what's going on, as it looks like the ethnl_phy_start() doesn't get
> > called.
>
> Make sure to have in your .config
>
> CONFIG_REF_TRACKER=y
> CONFIG_NET_DEV_REFCNT_TRACKER=y
> CONFIG_NET_NS_REFCNT_TRACKER=y
Good point, I now reproduce the issue indeed. Thanks a lot for the tips,
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit
2024-09-11 8:00 [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done syzbot
2024-09-11 10:04 ` Maxime Chevallier
@ 2024-09-13 8:07 ` Lizhi Xu
2024-09-13 11:44 ` Simon Horman
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Lizhi Xu @ 2024-09-13 8:07 UTC (permalink / raw)
To: syzbot+e9ed4e4368d450c8f9db
Cc: christophe.leroy, davem, edumazet, kuba, linux-kernel,
maxime.chevallier, netdev, pabeni, syzkaller-bugs
Syzbot reported a refcount bug in ethnl_phy_done.
This is because when executing ethnl_phy_done, it does not know who obtained
the dev(it can be got by ethnl_phy_doit or ethnl_phy_start) and directly
executes ethnl_parse_header_dev_put as long as the dev is not NULL.
Add dev_start_doit to the structure phy_req_info to distinguish who obtains dev.
Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
Reported-and-tested-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
net/ethtool/phy.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index 4ef7c6e32d10..321a7f89803f 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -13,6 +13,7 @@
struct phy_req_info {
struct ethnl_req_info base;
struct phy_device_node *pdn;
+ u8 dev_start_doit;
};
#define PHY_REQINFO(__req_base) \
@@ -157,6 +158,9 @@ int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
if (ret < 0)
return ret;
+ if (req_info.base.dev)
+ req_info.dev_start_doit = 0;
+
rtnl_lock();
ret = ethnl_phy_parse_request(&req_info.base, tb, info->extack);
@@ -223,10 +227,14 @@ int ethnl_phy_start(struct netlink_callback *cb)
false);
ctx->ifindex = 0;
ctx->phy_index = 0;
+ ctx->phy_req_info->dev_start_doit = 0;
if (ret)
kfree(ctx->phy_req_info);
+ if (ctx->phy_req_info->base.dev)
+ ctx->phy_req_info->dev_start_doit = 1;
+
return ret;
}
@@ -234,7 +242,7 @@ int ethnl_phy_done(struct netlink_callback *cb)
{
struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
- if (ctx->phy_req_info->base.dev)
+ if (ctx->phy_req_info->base.dev && ctx->phy_req_info->dev_start_doit)
ethnl_parse_header_dev_put(&ctx->phy_req_info->base);
kfree(ctx->phy_req_info);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
@ 2024-09-13 11:44 ` Simon Horman
2024-09-13 11:51 ` Maxime Chevallier
2024-09-16 7:38 ` Dan Carpenter
2 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2024-09-13 11:44 UTC (permalink / raw)
To: Lizhi Xu
Cc: syzbot+e9ed4e4368d450c8f9db, christophe.leroy, davem, edumazet,
kuba, linux-kernel, maxime.chevallier, netdev, pabeni,
syzkaller-bugs
On Fri, Sep 13, 2024 at 04:07:13PM +0800, Lizhi Xu wrote:
> Syzbot reported a refcount bug in ethnl_phy_done.
> This is because when executing ethnl_phy_done, it does not know who obtained
> the dev(it can be got by ethnl_phy_doit or ethnl_phy_start) and directly
> executes ethnl_parse_header_dev_put as long as the dev is not NULL.
> Add dev_start_doit to the structure phy_req_info to distinguish who obtains dev.
>
> Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
> Reported-and-tested-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
It seems that Maxime has also posted a patch for this problem.
- [PATCH net-next] net: ethtool: phy: Don't set the context dev pointer for unfiltered DUMP
https://lore.kernel.org/all/20240913100515.167341-1-maxime.chevallier@bootlin.com/
> ---
> net/ethtool/phy.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
> index 4ef7c6e32d10..321a7f89803f 100644
> --- a/net/ethtool/phy.c
> +++ b/net/ethtool/phy.c
> @@ -13,6 +13,7 @@
> struct phy_req_info {
> struct ethnl_req_info base;
> struct phy_device_node *pdn;
> + u8 dev_start_doit;
I think bool might be a more suitable type for this field.
> };
>
> #define PHY_REQINFO(__req_base) \
> @@ -157,6 +158,9 @@ int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
> if (ret < 0)
> return ret;
>
> + if (req_info.base.dev)
> + req_info.dev_start_doit = 0;
> +
> rtnl_lock();
>
> ret = ethnl_phy_parse_request(&req_info.base, tb, info->extack);
> @@ -223,10 +227,14 @@ int ethnl_phy_start(struct netlink_callback *cb)
> false);
> ctx->ifindex = 0;
> ctx->phy_index = 0;
> + ctx->phy_req_info->dev_start_doit = 0;
>
> if (ret)
> kfree(ctx->phy_req_info);
>
> + if (ctx->phy_req_info->base.dev)
> + ctx->phy_req_info->dev_start_doit = 1;
This doesn't seem right, ctx->phy_req_info may have been freed above.
> +
> return ret;
> }
>
> @@ -234,7 +242,7 @@ int ethnl_phy_done(struct netlink_callback *cb)
> {
> struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
>
> - if (ctx->phy_req_info->base.dev)
> + if (ctx->phy_req_info->base.dev && ctx->phy_req_info->dev_start_doit)
> ethnl_parse_header_dev_put(&ctx->phy_req_info->base);
>
> kfree(ctx->phy_req_info);
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
2024-09-13 11:44 ` Simon Horman
@ 2024-09-13 11:51 ` Maxime Chevallier
2024-09-16 7:38 ` Dan Carpenter
2 siblings, 0 replies; 8+ messages in thread
From: Maxime Chevallier @ 2024-09-13 11:51 UTC (permalink / raw)
To: Lizhi Xu
Cc: syzbot+e9ed4e4368d450c8f9db, christophe.leroy, davem, edumazet,
kuba, linux-kernel, netdev, pabeni, syzkaller-bugs
Hi,
On Fri, 13 Sep 2024 16:07:13 +0800
Lizhi Xu <lizhi.xu@windriver.com> wrote:
> Syzbot reported a refcount bug in ethnl_phy_done.
> This is because when executing ethnl_phy_done, it does not know who obtained
> the dev(it can be got by ethnl_phy_doit or ethnl_phy_start) and directly
> executes ethnl_parse_header_dev_put as long as the dev is not NULL.
> Add dev_start_doit to the structure phy_req_info to distinguish who obtains dev.
>
> Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface")
> Reported-and-tested-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e9ed4e4368d450c8f9db
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Thanks for addressing this, however I've already sent a first fix for
this [1] yesterday, followed-up by a second one [2] with another
approach following the reviews.
[1] : https://lore.kernel.org/netdev/20240913091404.3d4a9d19@fedora.home/T/#m4777416dbe26bf97b3a0a323fc71a93b40e0f7fb
[2] : https://lore.kernel.org/netdev/20240913100515.167341-1-maxime.chevallier@bootlin.com/T/#u
Best regards,
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
2024-09-13 11:44 ` Simon Horman
2024-09-13 11:51 ` Maxime Chevallier
@ 2024-09-16 7:38 ` Dan Carpenter
2 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2024-09-16 7:38 UTC (permalink / raw)
To: oe-kbuild, Lizhi Xu, syzbot+e9ed4e4368d450c8f9db
Cc: lkp, oe-kbuild-all, christophe.leroy, davem, edumazet, kuba,
linux-kernel, maxime.chevallier, netdev, pabeni, syzkaller-bugs
Hi Lizhi,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/net-ethtool-phy-Distinguish-whether-dev-is-got-by-phy-start-or-doit/20240913-160835
base: net-next/main
patch link: https://lore.kernel.org/r/20240913080714.1809254-1-lizhi.xu%40windriver.com
patch subject: [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit
config: x86_64-randconfig-r072-20240914 (https://download.01.org/0day-ci/archive/20240916/202409161017.tjjHpXGT-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202409161017.tjjHpXGT-lkp@intel.com/
smatch warnings:
net/ethtool/phy.c:235 ethnl_phy_start() error: dereferencing freed memory 'ctx->phy_req_info'
vim +235 net/ethtool/phy.c
17194be4c8e1e8 Maxime Chevallier 2024-08-21 212 int ethnl_phy_start(struct netlink_callback *cb)
17194be4c8e1e8 Maxime Chevallier 2024-08-21 213 {
17194be4c8e1e8 Maxime Chevallier 2024-08-21 214 const struct genl_info *info = genl_info_dump(cb);
17194be4c8e1e8 Maxime Chevallier 2024-08-21 215 struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 216 int ret;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 217
17194be4c8e1e8 Maxime Chevallier 2024-08-21 218 BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
17194be4c8e1e8 Maxime Chevallier 2024-08-21 219
17194be4c8e1e8 Maxime Chevallier 2024-08-21 220 ctx->phy_req_info = kzalloc(sizeof(*ctx->phy_req_info), GFP_KERNEL);
17194be4c8e1e8 Maxime Chevallier 2024-08-21 221 if (!ctx->phy_req_info)
17194be4c8e1e8 Maxime Chevallier 2024-08-21 222 return -ENOMEM;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 223
17194be4c8e1e8 Maxime Chevallier 2024-08-21 224 ret = ethnl_parse_header_dev_get(&ctx->phy_req_info->base,
17194be4c8e1e8 Maxime Chevallier 2024-08-21 225 info->attrs[ETHTOOL_A_PHY_HEADER],
17194be4c8e1e8 Maxime Chevallier 2024-08-21 226 sock_net(cb->skb->sk), cb->extack,
17194be4c8e1e8 Maxime Chevallier 2024-08-21 227 false);
17194be4c8e1e8 Maxime Chevallier 2024-08-21 228 ctx->ifindex = 0;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 229 ctx->phy_index = 0;
355b18bd0d5516 Lizhi Xu 2024-09-13 230 ctx->phy_req_info->dev_start_doit = 0;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 231
17194be4c8e1e8 Maxime Chevallier 2024-08-21 232 if (ret)
17194be4c8e1e8 Maxime Chevallier 2024-08-21 233 kfree(ctx->phy_req_info);
^^^^^^^^^^^^^^^^^
Freed
17194be4c8e1e8 Maxime Chevallier 2024-08-21 234
355b18bd0d5516 Lizhi Xu 2024-09-13 @235 if (ctx->phy_req_info->base.dev)
^^^^^^^^^^^^^^^^^
Use after free
355b18bd0d5516 Lizhi Xu 2024-09-13 236 ctx->phy_req_info->dev_start_doit = 1;
355b18bd0d5516 Lizhi Xu 2024-09-13 237
17194be4c8e1e8 Maxime Chevallier 2024-08-21 238 return ret;
17194be4c8e1e8 Maxime Chevallier 2024-08-21 239 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-09-16 7:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-11 8:00 [syzbot] [net?] WARNING: refcount bug in ethnl_phy_done syzbot
2024-09-11 10:04 ` Maxime Chevallier
2024-09-11 10:08 ` Eric Dumazet
2024-09-11 11:40 ` Maxime Chevallier
2024-09-13 8:07 ` [PATCH net-next] net: ethtool: phy: Distinguish whether dev is got by phy start or doit Lizhi Xu
2024-09-13 11:44 ` Simon Horman
2024-09-13 11:51 ` Maxime Chevallier
2024-09-16 7:38 ` Dan Carpenter
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).