| Message ID | 20240522031537.51741-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1,net-next] ila: avoid genlmsg_reply when not ila_map found | expand |
Wed, May 22, 2024 at 05:15:37AM CEST, linma@zju.edu.cn wrote: >The current ila_xlat_nl_cmd_get_mapping will call genlmsg_reply even if >not ila_map found with user provided parameters. Then an empty netlink >message will be sent and cause a WARNING like below. > >WARNING: CPU: 1 PID: 7709 at include/linux/skbuff.h:2524 __dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 >Modules linked in: >Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >RIP: 0010:skb_assert_len include/linux/skbuff.h:2524 [inline] >RIP: 0010:__dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 >RSP: 0018:ffffc9000f90f228 EFLAGS: 00010282 >RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 >RDX: 0000000000040000 RSI: ffffffff816968a8 RDI: fffff52001f21e37 >RBP: ffff8881077f2d3a R08: 0000000000000005 R09: 0000000000000000 >R10: 0000000080000000 R11: 0000000000000000 R12: dffffc0000000000 >R13: 0000000000000000 R14: ffff8881077f2c90 R15: ffff8881077f2c80 >FS: 00007fb8be338700(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000 >CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >CR2: 000000c00ca8a000 CR3: 0000000105e17000 CR4: 0000000000150ee0 >Call Trace: > <TASK> > dev_queue_xmit include/linux/netdevice.h:3008 [inline] > __netlink_deliver_tap_skb net/netlink/af_netlink.c:307 [inline] > __netlink_deliver_tap net/netlink/af_netlink.c:325 [inline] > netlink_deliver_tap+0x9e4/0xc40 net/netlink/af_netlink.c:338 > __netlink_sendskb net/netlink/af_netlink.c:1263 [inline] > netlink_sendskb net/netlink/af_netlink.c:1272 [inline] > netlink_unicast+0x6ac/0x7f0 net/netlink/af_netlink.c:1360 > nlmsg_unicast include/net/netlink.h:1067 [inline] > genlmsg_unicast include/net/genetlink.h:372 [inline] > genlmsg_reply include/net/genetlink.h:382 [inline] > ila_xlat_nl_cmd_get_mapping+0x589/0x950 net/ipv6/ila/ila_xlat.c:493 > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:756 > genl_family_rcv_msg net/netlink/genetlink.c:833 [inline] > genl_rcv_msg+0x441/0x780 net/netlink/genetlink.c:850 > netlink_rcv_skb+0x153/0x400 net/netlink/af_netlink.c:2540 > genl_rcv+0x24/0x40 net/netlink/genetlink.c:861 > netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline] > netlink_unicast+0x543/0x7f0 net/netlink/af_netlink.c:1345 > netlink_sendmsg+0x917/0xe10 net/netlink/af_netlink.c:1921 > sock_sendmsg_nosec net/socket.c:714 [inline] > sock_sendmsg+0xcf/0x120 net/socket.c:734 > ____sys_sendmsg+0x6eb/0x810 net/socket.c:2482 > ___sys_sendmsg+0x110/0x1b0 net/socket.c:2536 > __sys_sendmsg+0xf3/0x1c0 net/socket.c:2565 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd >RIP: 0033:0x7fb8bd68f359 >Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 19 00 00 90 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:00007fb8be338168 EFLAGS: 00000246 ORIG_RAX: 000000000000002e >RAX: ffffffffffffffda RBX: 00007fb8bd7bbf80 RCX: 00007fb8bd68f359 >RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000005 >RBP: 00007fb8bd6da498 R08: 0000000000000000 R09: 0000000000000000 >R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 >R13: 00007ffc22fb52af R14: 00007fb8be338300 R15: 0000000000022000 > >Do nullptr check and assign -EINVAL ret if no ila_map found. > >Signed-off-by: Lin Ma <linma@zju.edu.cn> You should aim -net tree and add appropriate Fixes tag here. >--- > net/ipv6/ila/ila_xlat.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c >index 67e8c9440977..63b8fe1b8255 100644 >--- a/net/ipv6/ila/ila_xlat.c >+++ b/net/ipv6/ila/ila_xlat.c >@@ -483,6 +483,8 @@ int ila_xlat_nl_cmd_get_mapping(struct sk_buff *skb, struct genl_info *info) > info->snd_portid, > info->snd_seq, 0, msg, > info->genlhdr->cmd); >+ } else { >+ ret = -EINVAL; > } > > rcu_read_unlock(); >-- >2.34.1 > >
On Wed, May 22, 2024 at 11:15:37AM +0800, Lin Ma wrote: > The current ila_xlat_nl_cmd_get_mapping will call genlmsg_reply even if > not ila_map found with user provided parameters. Then an empty netlink > message will be sent and cause a WARNING like below. > > WARNING: CPU: 1 PID: 7709 at include/linux/skbuff.h:2524 __dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 > Modules linked in: > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > RIP: 0010:skb_assert_len include/linux/skbuff.h:2524 [inline] > RIP: 0010:__dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 > RSP: 0018:ffffc9000f90f228 EFLAGS: 00010282 > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 0000000000040000 RSI: ffffffff816968a8 RDI: fffff52001f21e37 > RBP: ffff8881077f2d3a R08: 0000000000000005 R09: 0000000000000000 > R10: 0000000080000000 R11: 0000000000000000 R12: dffffc0000000000 > R13: 0000000000000000 R14: ffff8881077f2c90 R15: ffff8881077f2c80 > FS: 00007fb8be338700(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c00ca8a000 CR3: 0000000105e17000 CR4: 0000000000150ee0 > Call Trace: > <TASK> > dev_queue_xmit include/linux/netdevice.h:3008 [inline] > __netlink_deliver_tap_skb net/netlink/af_netlink.c:307 [inline] > __netlink_deliver_tap net/netlink/af_netlink.c:325 [inline] > netlink_deliver_tap+0x9e4/0xc40 net/netlink/af_netlink.c:338 > __netlink_sendskb net/netlink/af_netlink.c:1263 [inline] > netlink_sendskb net/netlink/af_netlink.c:1272 [inline] > netlink_unicast+0x6ac/0x7f0 net/netlink/af_netlink.c:1360 > nlmsg_unicast include/net/netlink.h:1067 [inline] > genlmsg_unicast include/net/genetlink.h:372 [inline] > genlmsg_reply include/net/genetlink.h:382 [inline] > ila_xlat_nl_cmd_get_mapping+0x589/0x950 net/ipv6/ila/ila_xlat.c:493 > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:756 > genl_family_rcv_msg net/netlink/genetlink.c:833 [inline] > genl_rcv_msg+0x441/0x780 net/netlink/genetlink.c:850 > netlink_rcv_skb+0x153/0x400 net/netlink/af_netlink.c:2540 > genl_rcv+0x24/0x40 net/netlink/genetlink.c:861 > netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline] > netlink_unicast+0x543/0x7f0 net/netlink/af_netlink.c:1345 > netlink_sendmsg+0x917/0xe10 net/netlink/af_netlink.c:1921 > sock_sendmsg_nosec net/socket.c:714 [inline] > sock_sendmsg+0xcf/0x120 net/socket.c:734 > ____sys_sendmsg+0x6eb/0x810 net/socket.c:2482 > ___sys_sendmsg+0x110/0x1b0 net/socket.c:2536 > __sys_sendmsg+0xf3/0x1c0 net/socket.c:2565 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7fb8bd68f359 > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 19 00 00 90 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:00007fb8be338168 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 00007fb8bd7bbf80 RCX: 00007fb8bd68f359 > RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000005 > RBP: 00007fb8bd6da498 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 > R13: 00007ffc22fb52af R14: 00007fb8be338300 R15: 0000000000022000 > > Do nullptr check and assign -EINVAL ret if no ila_map found. > > Signed-off-by: Lin Ma <linma@zju.edu.cn> > --- > net/ipv6/ila/ila_xlat.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c > index 67e8c9440977..63b8fe1b8255 100644 > --- a/net/ipv6/ila/ila_xlat.c > +++ b/net/ipv6/ila/ila_xlat.c Hi Lin Ma, The lines immediately above those covered by this patch are as follows: ret = -ESRCH; ila = ila_lookup_by_params(&xp, ilan); if (ila) { ret = ila_dump_info(ila, > @@ -483,6 +483,8 @@ int ila_xlat_nl_cmd_get_mapping(struct sk_buff *skb, struct genl_info *info) > info->snd_portid, > info->snd_seq, 0, msg, > info->genlhdr->cmd); > + } else { > + ret = -EINVAL; > } > > rcu_read_unlock(); And the lines following, up to the end of the function, are: if (ret < 0) goto out_free; return genlmsg_reply(msg, info); out_free: nlmsg_free(msg); return ret; By my reading, without your patch, if ila is not found (NULL) then ret will be -ESRCH, and genlmsg_reply will not be called. I feel that I am missing something here.
Hi Simon. > > Hi Lin Ma, > > The lines immediately above those covered by this patch are as follows: > > ret = -ESRCH; > ila = ila_lookup_by_params(&xp, ilan); > if (ila) { > ret = ila_dump_info(ila, > > > @@ -483,6 +483,8 @@ int ila_xlat_nl_cmd_get_mapping(struct sk_buff *skb, struct genl_info *info) > > info->snd_portid, > > info->snd_seq, 0, msg, > > info->genlhdr->cmd); > > + } else { > > + ret = -EINVAL; > > } > > > > rcu_read_unlock(); > > And the lines following, up to the end of the function, are: > > if (ret < 0) > goto out_free; > > return genlmsg_reply(msg, info); > > out_free: > nlmsg_free(msg); > return ret; > > By my reading, without your patch, if ila is not found (NULL) > then ret will be -ESRCH, and genlmsg_reply will not be called. > > I feel that I am missing something here. Oh my bad, it seems this bug was already fixed by the commit 693aa2c0d9b6 ("ila: do not generate empty messages in ila_xlat_nl_cmd_get_mapping()") last year. And my dated kernel does not apply that one. Thanks for reminding me of this false alarm. Regards Lin
On Thu, May 23, 2024 at 09:44:15AM +0800, Lin Ma wrote: > Hi Simon. > > > > > Hi Lin Ma, > > > > The lines immediately above those covered by this patch are as follows: > > > > ret = -ESRCH; > > ila = ila_lookup_by_params(&xp, ilan); > > if (ila) { > > ret = ila_dump_info(ila, > > > > > @@ -483,6 +483,8 @@ int ila_xlat_nl_cmd_get_mapping(struct sk_buff *skb, struct genl_info *info) > > > info->snd_portid, > > > info->snd_seq, 0, msg, > > > info->genlhdr->cmd); > > > + } else { > > > + ret = -EINVAL; > > > } > > > > > > rcu_read_unlock(); > > > > And the lines following, up to the end of the function, are: > > > > if (ret < 0) > > goto out_free; > > > > return genlmsg_reply(msg, info); > > > > out_free: > > nlmsg_free(msg); > > return ret; > > > > By my reading, without your patch, if ila is not found (NULL) > > then ret will be -ESRCH, and genlmsg_reply will not be called. > > > > I feel that I am missing something here. > > Oh my bad, it seems this bug was already fixed by the > commit 693aa2c0d9b6 ("ila: do not generate empty messages > in ila_xlat_nl_cmd_get_mapping()") last year. > And my dated kernel does not apply that one. > > Thanks for reminding me of this false alarm. Thanks for checking. I think we can retire this patch.
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c index 67e8c9440977..63b8fe1b8255 100644 --- a/net/ipv6/ila/ila_xlat.c +++ b/net/ipv6/ila/ila_xlat.c @@ -483,6 +483,8 @@ int ila_xlat_nl_cmd_get_mapping(struct sk_buff *skb, struct genl_info *info) info->snd_portid, info->snd_seq, 0, msg, info->genlhdr->cmd); + } else { + ret = -EINVAL; } rcu_read_unlock();
The current ila_xlat_nl_cmd_get_mapping will call genlmsg_reply even if not ila_map found with user provided parameters. Then an empty netlink message will be sent and cause a WARNING like below. WARNING: CPU: 1 PID: 7709 at include/linux/skbuff.h:2524 __dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 Modules linked in: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 RIP: 0010:skb_assert_len include/linux/skbuff.h:2524 [inline] RIP: 0010:__dev_queue_xmit+0x241b/0x3b60 net/core/dev.c:4171 RSP: 0018:ffffc9000f90f228 EFLAGS: 00010282 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000040000 RSI: ffffffff816968a8 RDI: fffff52001f21e37 RBP: ffff8881077f2d3a R08: 0000000000000005 R09: 0000000000000000 R10: 0000000080000000 R11: 0000000000000000 R12: dffffc0000000000 R13: 0000000000000000 R14: ffff8881077f2c90 R15: ffff8881077f2c80 FS: 00007fb8be338700(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000000c00ca8a000 CR3: 0000000105e17000 CR4: 0000000000150ee0 Call Trace: <TASK> dev_queue_xmit include/linux/netdevice.h:3008 [inline] __netlink_deliver_tap_skb net/netlink/af_netlink.c:307 [inline] __netlink_deliver_tap net/netlink/af_netlink.c:325 [inline] netlink_deliver_tap+0x9e4/0xc40 net/netlink/af_netlink.c:338 __netlink_sendskb net/netlink/af_netlink.c:1263 [inline] netlink_sendskb net/netlink/af_netlink.c:1272 [inline] netlink_unicast+0x6ac/0x7f0 net/netlink/af_netlink.c:1360 nlmsg_unicast include/net/netlink.h:1067 [inline] genlmsg_unicast include/net/genetlink.h:372 [inline] genlmsg_reply include/net/genetlink.h:382 [inline] ila_xlat_nl_cmd_get_mapping+0x589/0x950 net/ipv6/ila/ila_xlat.c:493 genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:756 genl_family_rcv_msg net/netlink/genetlink.c:833 [inline] genl_rcv_msg+0x441/0x780 net/netlink/genetlink.c:850 netlink_rcv_skb+0x153/0x400 net/netlink/af_netlink.c:2540 genl_rcv+0x24/0x40 net/netlink/genetlink.c:861 netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline] netlink_unicast+0x543/0x7f0 net/netlink/af_netlink.c:1345 netlink_sendmsg+0x917/0xe10 net/netlink/af_netlink.c:1921 sock_sendmsg_nosec net/socket.c:714 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:734 ____sys_sendmsg+0x6eb/0x810 net/socket.c:2482 ___sys_sendmsg+0x110/0x1b0 net/socket.c:2536 __sys_sendmsg+0xf3/0x1c0 net/socket.c:2565 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7fb8bd68f359 Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 19 00 00 90 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:00007fb8be338168 EFLAGS: 00000246 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00007fb8bd7bbf80 RCX: 00007fb8bd68f359 RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000005 RBP: 00007fb8bd6da498 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 00007ffc22fb52af R14: 00007fb8be338300 R15: 0000000000022000 Do nullptr check and assign -EINVAL ret if no ila_map found. Signed-off-by: Lin Ma <linma@zju.edu.cn> --- net/ipv6/ila/ila_xlat.c | 2 ++ 1 file changed, 2 insertions(+)