[PATCH net] netlink: do not reset transport header in netlink_recvmsg()

5 views
Skip to first unread message

Eric Dumazet

unread,
May 5, 2022, 12:19:52 PM5/5/22
to David S . Miller, Jakub Kicinski, Paolo Abeni, netdev, Eric Dumazet, Eric Dumazet, syzbot
From: Eric Dumazet <edum...@google.com>

netlink_recvmsg() does not need to change transport header.

If transport header was needed, it should have been reset
by the producer (netlink_dump()), not the consumer(s).

The following trace probably happened when multiple threads
were using MSG_PEEK.

BUG: KCSAN: data-race in netlink_recvmsg / netlink_recvmsg

write to 0xffff88811e9f15b2 of 2 bytes by task 32012 on cpu 1:
skb_reset_transport_header include/linux/skbuff.h:2760 [inline]
netlink_recvmsg+0x1de/0x790 net/netlink/af_netlink.c:1978
sock_recvmsg_nosec net/socket.c:948 [inline]
sock_recvmsg net/socket.c:966 [inline]
__sys_recvfrom+0x204/0x2c0 net/socket.c:2097
__do_sys_recvfrom net/socket.c:2115 [inline]
__se_sys_recvfrom net/socket.c:2111 [inline]
__x64_sys_recvfrom+0x74/0x90 net/socket.c:2111
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

write to 0xffff88811e9f15b2 of 2 bytes by task 32005 on cpu 0:
skb_reset_transport_header include/linux/skbuff.h:2760 [inline]
netlink_recvmsg+0x1de/0x790 net/netlink/af_netlink.c:1978
____sys_recvmsg+0x162/0x2f0
___sys_recvmsg net/socket.c:2674 [inline]
__sys_recvmsg+0x209/0x3f0 net/socket.c:2704
__do_sys_recvmsg net/socket.c:2714 [inline]
__se_sys_recvmsg net/socket.c:2711 [inline]
__x64_sys_recvmsg+0x42/0x50 net/socket.c:2711
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

value changed: 0xffff -> 0x0000

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 32005 Comm: syz-executor.4 Not tainted 5.18.0-rc1-syzkaller-00328-ge1f700ebd6be-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edum...@google.com>
Reported-by: syzbot <syzk...@googlegroups.com>
---
net/netlink/af_netlink.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 05a3795eac8e9a7c8343460d9a41e0755a64c36e..73e9c0a9c187674cced15dbec079734489c3329f 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1975,7 +1975,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
copied = len;
}

- skb_reset_transport_header(data_skb);
err = skb_copy_datagram_msg(data_skb, 0, msg, copied);

if (msg->msg_name) {
--
2.36.0.512.ge40c2bad7a-goog

Jakub Kicinski

unread,
May 5, 2022, 12:57:56 PM5/5/22
to Eric Dumazet, David S . Miller, Paolo Abeni, netdev, Eric Dumazet, syzbot
On Thu, 5 May 2022 09:19:46 -0700 Eric Dumazet wrote:
> From: Eric Dumazet <edum...@google.com>
>
> netlink_recvmsg() does not need to change transport header.
>
> If transport header was needed, it should have been reset
> by the producer (netlink_dump()), not the consumer(s).

Should I insert a reference to commit 99c07327ae11 ("netlink: reset
network and mac headers in netlink_dump()") when applying to give
backporters an extra hint?

Eric Dumazet

unread,
May 5, 2022, 1:05:00 PM5/5/22
to Jakub Kicinski, Eric Dumazet, David S . Miller, Paolo Abeni, netdev, syzbot
I thought about that, but CBPF has no business with transport header.

I felt this would confuse things.
 

Eric Dumazet

unread,
May 5, 2022, 1:06:00 PM5/5/22
to Jakub Kicinski, Eric Dumazet, David S . Miller, Paolo Abeni, netdev, syzbot
On Thu, May 5, 2022 at 9:57 AM Jakub Kicinski <ku...@kernel.org> wrote:
>

patchwork-b...@kernel.org

unread,
May 6, 2022, 6:50:14 PM5/6/22
to Eric Dumazet, da...@davemloft.net, ku...@kernel.org, pab...@redhat.com, net...@vger.kernel.org, edum...@google.com, syzk...@googlegroups.com
Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <ku...@kernel.org>:

On Thu, 5 May 2022 09:19:46 -0700 you wrote:
> From: Eric Dumazet <edum...@google.com>
>
> netlink_recvmsg() does not need to change transport header.
>
> If transport header was needed, it should have been reset
> by the producer (netlink_dump()), not the consumer(s).
>
> [...]

Here is the summary with links:
- [net] netlink: do not reset transport header in netlink_recvmsg()
https://git.kernel.org/netdev/net/c/d5076fe4049c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


Greg Kroah-Hartman

unread,
May 16, 2022, 3:38:35 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
From: Eric Dumazet <edum...@google.com>

[ Upstream commit d5076fe4049cadef1f040eda4aaa001bb5424225 ]

netlink_recvmsg() does not need to change transport header.

If transport header was needed, it should have been reset
by the producer (netlink_dump()), not the consumer(s).

Link: https://lore.kernel.org/r/20220505161946.286...@gmail.com
Signed-off-by: Jakub Kicinski <ku...@kernel.org>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
net/netlink/af_netlink.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index a8674e9ff37b..47b1631bf14c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1929,7 +1929,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
copied = len;
}

- skb_reset_transport_header(data_skb);
err = skb_copy_datagram_msg(data_skb, 0, msg, copied);

if (msg->msg_name) {
--
2.35.1



Greg Kroah-Hartman

unread,
May 16, 2022, 3:40:10 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index 979cd7dff40a..1b2e99ce54e5 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1947,7 +1947,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

Greg Kroah-Hartman

unread,
May 16, 2022, 3:41:18 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index e2120221b957..6ffa83319d08 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1978,7 +1978,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

Greg Kroah-Hartman

unread,
May 16, 2022, 3:43:13 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index 8aefc52542a0..86b70385dce3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1987,7 +1987,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

Greg Kroah-Hartman

unread,
May 16, 2022, 3:44:34 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index cbfb601c4ee9..d96a610929d9 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1988,7 +1988,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

Greg Kroah-Hartman

unread,
May 16, 2022, 3:48:20 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index fb7f7b17c78c..974d32632ef4 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1996,7 +1996,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

Greg Kroah-Hartman

unread,
May 16, 2022, 3:53:57 PM5/16/22
to linux-...@vger.kernel.org, Greg Kroah-Hartman, sta...@vger.kernel.org, Eric Dumazet, syzbot, Jakub Kicinski, Sasha Levin
index 05a3795eac8e..73e9c0a9c187 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1975,7 +1975,6 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
Reply all
Reply to author
Forward
0 new messages