* [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
@ 2025-03-31 8:10 Wang Liang
2025-04-01 11:01 ` Paolo Abeni
2025-04-03 11:55 ` Wenjia Zhang
0 siblings, 2 replies; 17+ messages in thread
From: Wang Liang @ 2025-03-31 8:10 UTC (permalink / raw)
To: wenjia, jaka, alibuda, tonylu, guwen, davem, edumazet, kuba,
pabeni, horms, ubraun
Cc: yuehaibing, zhangchangzhong, wangliang74, linux-rdma, linux-s390,
netdev, linux-kernel
Syzbot reported a general protection fault:
CPU: 0 UID: 0 PID: 5830 Comm: syz-executor600 Not tainted 6.14.0-rc4-syzkaller-00090-gdd83757f6e68 #0
RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
RIP: 0010:__smc_diag_dump.constprop.0+0x3de/0x23d0 net/smc/smc_diag.c:89
Call Trace:
<TASK>
smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
smc_diag_dump+0x84/0x90 net/smc/smc_diag.c:236
netlink_dump+0x53c/0xd00 net/netlink/af_netlink.c:2318
__netlink_dump_start+0x6ca/0x970 net/netlink/af_netlink.c:2433
netlink_dump_start include/linux/netlink.h:340 [inline]
smc_diag_handler_dump+0x1fb/0x240 net/smc/smc_diag.c:251
__sock_diag_cmd net/core/sock_diag.c:249 [inline]
sock_diag_rcv_msg+0x437/0x790 net/core/sock_diag.c:287
netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2543
netlink_unicast_kernel net/netlink/af_netlink.c:1322 [inline]
netlink_unicast+0x53c/0x7f0 net/netlink/af_netlink.c:1348
netlink_sendmsg+0x8b8/0xd70 net/netlink/af_netlink.c:1892
sock_sendmsg_nosec net/socket.c:718 [inline]
__sock_sendmsg net/socket.c:733 [inline]
____sys_sendmsg+0xaaf/0xc90 net/socket.c:2573
___sys_sendmsg+0x135/0x1e0 net/socket.c:2627
__sys_sendmsg+0x16e/0x220 net/socket.c:2659
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
When create smc socket, smc_inet_init_sock() first add sk to the smc_hash
by smc_hash_sk(), then create smc->clcsock. it is possible that, after
smc_diag_dump_proto() traverses the smc_hash, smc->clcsock is not created
when the function visit it.
The process like this:
(CPU1) | (CPU2)
inet6_create() |
smc_inet_init_sock() |
smc_sk_init() |
smc_hash_sk() |
head = &smc_hash->ht; |
sk_add_node(sk, head); |
| smc_diag_dump_proto
| head = &smc_hash->ht;
| sk_for_each(sk, head)
| __smc_diag_dump()
| visit smc->clcsock
smc_create_clcsk() |
set smc->clcsock |
Fix this by initialize smc->clcsock to NULL before add sk to smc_hash in
smc_sk_init().
Reported-by: syzbot+271fed3ed6f24600c364@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
Signed-off-by: Wang Liang <wangliang74@huawei.com>
---
net/smc/af_smc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 3e6cb35baf25..454801188514 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
sk->sk_protocol = protocol;
WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
+ smc->clcsock = NULL;
INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
INIT_WORK(&smc->connect_work, smc_connect_work);
INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-03-31 8:10 [PATCH net] net/smc: fix general protection fault in __smc_diag_dump Wang Liang
@ 2025-04-01 11:01 ` Paolo Abeni
2025-04-01 13:31 ` Zhu Yanjun
2025-04-02 2:37 ` Wang Liang
2025-04-03 11:55 ` Wenjia Zhang
1 sibling, 2 replies; 17+ messages in thread
From: Paolo Abeni @ 2025-04-01 11:01 UTC (permalink / raw)
To: Wang Liang, wenjia, jaka, alibuda, tonylu, guwen, davem, edumazet,
kuba, horms, ubraun
Cc: yuehaibing, zhangchangzhong, linux-rdma, linux-s390, netdev,
linux-kernel
On 3/31/25 10:10 AM, Wang Liang wrote:
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 3e6cb35baf25..454801188514 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
> sk->sk_protocol = protocol;
> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
> + smc->clcsock = NULL;
> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
> INIT_WORK(&smc->connect_work, smc_connect_work);
> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
The syzkaller report has a few reproducers, have you tested this? AFAICS
the smc socket is already zeroed on allocation by sk_alloc().
/P
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-04-01 11:01 ` Paolo Abeni
@ 2025-04-01 13:31 ` Zhu Yanjun
2025-04-02 2:37 ` Wang Liang
1 sibling, 0 replies; 17+ messages in thread
From: Zhu Yanjun @ 2025-04-01 13:31 UTC (permalink / raw)
To: Paolo Abeni, Wang Liang, wenjia, jaka, alibuda, tonylu, guwen,
davem, edumazet, kuba, horms, ubraun
Cc: yuehaibing, zhangchangzhong, linux-rdma, linux-s390, netdev,
linux-kernel
On 01.04.25 13:01, Paolo Abeni wrote:
> On 3/31/25 10:10 AM, Wang Liang wrote:
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index 3e6cb35baf25..454801188514 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
>> sk->sk_protocol = protocol;
>> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
>> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
>> + smc->clcsock = NULL;
>> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
>> INIT_WORK(&smc->connect_work, smc_connect_work);
>> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
>
> The syzkaller report has a few reproducers, have you tested this? AFAICS
> the smc socket is already zeroed on allocation by sk_alloc().
Yes. I also agree with you that smc socket should have already been zeroed.
Currently in this commit, this member variable is set to NULL
explicitly. I am not sure if this can fix this problem or not.
Based on the following, it seems that this problem can be reproduced.
"
syzbot has tested the proposed patch but the reproducer is still
triggering an issue:
general protection fault in __smc_diag_dump
"
Thus follow the instructions in this link to make tests.
https://groups.google.com/g/syzkaller-bugs/c/YwENRImdcsk/m/wBJo6qGiCAAJ?pli=1,
the following can trigger the reproducer.
"
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.
"
Zhu Yanjun
>
> /P
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-04-01 11:01 ` Paolo Abeni
2025-04-01 13:31 ` Zhu Yanjun
@ 2025-04-02 2:37 ` Wang Liang
2025-04-02 7:20 ` D. Wythe
1 sibling, 1 reply; 17+ messages in thread
From: Wang Liang @ 2025-04-02 2:37 UTC (permalink / raw)
To: Paolo Abeni, wenjia, jaka, alibuda, tonylu, guwen, davem,
edumazet, kuba, horms, ubraun, yanjun.zhu
Cc: yuehaibing, zhangchangzhong, linux-rdma, linux-s390, netdev,
linux-kernel
在 2025/4/1 19:01, Paolo Abeni 写道:
> On 3/31/25 10:10 AM, Wang Liang wrote:
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index 3e6cb35baf25..454801188514 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
>> sk->sk_protocol = protocol;
>> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
>> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
>> + smc->clcsock = NULL;
>> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
>> INIT_WORK(&smc->connect_work, smc_connect_work);
>> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
> The syzkaller report has a few reproducers, have you tested this? AFAICS
> the smc socket is already zeroed on allocation by sk_alloc().
Yes, I test it by the C repro:
https://syzkaller.appspot.com/text?tag=ReproC&x=13d2dc98580000
The C repro is provided by the 2025/02/27 15:16 crash from
https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
After apply my patch, the crash no longer happens when running the C repro.
"the smc socket is already zeroed on allocation by sk_alloc()", That is
right.
However, smc->clcsock may be modified indirectly in inet6_create().
The process like this:
__sys_socket
__sys_socket_create
sock_create
__sock_create
# pf->create
inet6_create
// init smc->clcsock = 0
sk = sk_alloc()
// set smc->clcsock to invalid address
inet = inet_sk(sk);
inet_assign_bit(IS_ICSK, sk, INET_PROTOSW_ICSK & answer_flags);
inet6_set_bit(MC6_LOOP, sk);
inet6_set_bit(MC6_ALL, sk);
smc_inet_init_sock
smc_sk_init
// add sk to smc_hash
smc_hash_sk
sk_add_node(sk, head);
smc_create_clcsk
// set smc->clcsock
sock_create_kern(..., &smc->clcsock);)
So initialize smc->clcsock to NULL explicitly in smc_sk_init() can fix
this crash scene. If the problem can be reproduced after this patch, I
guess it is not the same reason, and fix it by another patch is more
appropriate.
>
> /P
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-04-02 2:37 ` Wang Liang
@ 2025-04-02 7:20 ` D. Wythe
2025-04-03 7:09 ` Wang Liang
0 siblings, 1 reply; 17+ messages in thread
From: D. Wythe @ 2025-04-02 7:20 UTC (permalink / raw)
To: Wang Liang
Cc: Paolo Abeni, wenjia, jaka, alibuda, tonylu, guwen, davem,
edumazet, kuba, horms, ubraun, yanjun.zhu, yuehaibing,
zhangchangzhong, linux-rdma, linux-s390, netdev, linux-kernel
On Wed, Apr 02, 2025 at 10:37:24AM +0800, Wang Liang wrote:
>
> 在 2025/4/1 19:01, Paolo Abeni 写道:
> >On 3/31/25 10:10 AM, Wang Liang wrote:
> >>diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> >>index 3e6cb35baf25..454801188514 100644
> >>--- a/net/smc/af_smc.c
> >>+++ b/net/smc/af_smc.c
> >>@@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
> >> sk->sk_protocol = protocol;
> >> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
> >> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
> >>+ smc->clcsock = NULL;
> >> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
> >> INIT_WORK(&smc->connect_work, smc_connect_work);
> >> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
> >The syzkaller report has a few reproducers, have you tested this? AFAICS
> >the smc socket is already zeroed on allocation by sk_alloc().
>
>
> Yes, I test it by the C repro:
> https://syzkaller.appspot.com/text?tag=ReproC&x=13d2dc98580000
>
> The C repro is provided by the 2025/02/27 15:16 crash from
> https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
>
> After apply my patch, the crash no longer happens when running the C repro.
>
> "the smc socket is already zeroed on allocation by sk_alloc()", That
> is right.
> However, smc->clcsock may be modified indirectly in inet6_create().
> The process like this:
>
> __sys_socket
> __sys_socket_create
> sock_create
> __sock_create
> # pf->create
> inet6_create
> // init smc->clcsock = 0
> sk = sk_alloc()
>
> // set smc->clcsock to invalid address
> inet = inet_sk(sk);
> inet_assign_bit(IS_ICSK, sk, INET_PROTOSW_ICSK & answer_flags);
> inet6_set_bit(MC6_LOOP, sk);
> inet6_set_bit(MC6_ALL, sk);
>
> smc_inet_init_sock
> smc_sk_init
> // add sk to smc_hash
> smc_hash_sk
> sk_add_node(sk, head);
> smc_create_clcsk
> // set smc->clcsock
> sock_create_kern(..., &smc->clcsock);)
>
> So initialize smc->clcsock to NULL explicitly in smc_sk_init() can fix
> this crash scene. If the problem can be reproduced after this patch, I
> guess it is not the same reason, and fix it by another patch is more
> appropriate.
>
This is actually because the current smc_sock is not an inet_sock,
leading to two modules simultaneously modifying the same offset in
memory but interpreting its structure differently. I previously proposed
embedding an inet(6)_sock at the beginning of smc_sock, but the
community had some objections...
I'm not sure on the community's current stance on this matter, but if a
fix is absolutely necessary, my recommendation would still be to embed
an inet(6)_sock within the smc_sock structure
D.
> >
> >/P
> >
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-04-02 7:20 ` D. Wythe
@ 2025-04-03 7:09 ` Wang Liang
0 siblings, 0 replies; 17+ messages in thread
From: Wang Liang @ 2025-04-03 7:09 UTC (permalink / raw)
To: D. Wythe
Cc: Paolo Abeni, wenjia, jaka, tonylu, guwen, davem, edumazet, kuba,
horms, ubraun, yanjun.zhu, yuehaibing, zhangchangzhong,
linux-rdma, linux-s390, netdev, linux-kernel
在 2025/4/2 15:20, D. Wythe 写道:
> On Wed, Apr 02, 2025 at 10:37:24AM +0800, Wang Liang wrote:
>> 在 2025/4/1 19:01, Paolo Abeni 写道:
>>> On 3/31/25 10:10 AM, Wang Liang wrote:
>>>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>>>> index 3e6cb35baf25..454801188514 100644
>>>> --- a/net/smc/af_smc.c
>>>> +++ b/net/smc/af_smc.c
>>>> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
>>>> sk->sk_protocol = protocol;
>>>> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
>>>> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
>>>> + smc->clcsock = NULL;
>>>> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
>>>> INIT_WORK(&smc->connect_work, smc_connect_work);
>>>> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
>>> The syzkaller report has a few reproducers, have you tested this? AFAICS
>>> the smc socket is already zeroed on allocation by sk_alloc().
>>
>> Yes, I test it by the C repro:
>> https://syzkaller.appspot.com/text?tag=ReproC&x=13d2dc98580000
>>
>> The C repro is provided by the 2025/02/27 15:16 crash from
>> https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
>>
>> After apply my patch, the crash no longer happens when running the C repro.
>>
>> "the smc socket is already zeroed on allocation by sk_alloc()", That
>> is right.
>> However, smc->clcsock may be modified indirectly in inet6_create().
>> The process like this:
>>
>> __sys_socket
>> __sys_socket_create
>> sock_create
>> __sock_create
>> # pf->create
>> inet6_create
>> // init smc->clcsock = 0
>> sk = sk_alloc()
>>
>> // set smc->clcsock to invalid address
>> inet = inet_sk(sk);
>> inet_assign_bit(IS_ICSK, sk, INET_PROTOSW_ICSK & answer_flags);
>> inet6_set_bit(MC6_LOOP, sk);
>> inet6_set_bit(MC6_ALL, sk);
>>
>> smc_inet_init_sock
>> smc_sk_init
>> // add sk to smc_hash
>> smc_hash_sk
>> sk_add_node(sk, head);
>> smc_create_clcsk
>> // set smc->clcsock
>> sock_create_kern(..., &smc->clcsock);)
>>
>> So initialize smc->clcsock to NULL explicitly in smc_sk_init() can fix
>> this crash scene. If the problem can be reproduced after this patch, I
>> guess it is not the same reason, and fix it by another patch is more
>> appropriate.
>>
> This is actually because the current smc_sock is not an inet_sock,
> leading to two modules simultaneously modifying the same offset in
> memory but interpreting its structure differently. I previously proposed
> embedding an inet(6)_sock at the beginning of smc_sock, but the
> community had some objections...
>
> I'm not sure on the community's current stance on this matter, but if a
> fix is absolutely necessary, my recommendation would still be to embed
> an inet(6)_sock within the smc_sock structure
>
> D.
At present, I think initializing the smc in smc_sk_init() may be the
most simple and effective method. :P
>
>>> /P
>>>
>>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-03-31 8:10 [PATCH net] net/smc: fix general protection fault in __smc_diag_dump Wang Liang
2025-04-01 11:01 ` Paolo Abeni
@ 2025-04-03 11:55 ` Wenjia Zhang
2025-04-10 3:11 ` Wang Liang
1 sibling, 1 reply; 17+ messages in thread
From: Wenjia Zhang @ 2025-04-03 11:55 UTC (permalink / raw)
To: Wang Liang, jaka, alibuda, tonylu, guwen, davem, edumazet, kuba,
pabeni, horms, ubraun, Sidraya Jayagond, Mahanta Jambigi
Cc: yuehaibing, zhangchangzhong, linux-rdma, linux-s390, netdev,
linux-kernel
On 31.03.25 10:10, Wang Liang wrote:
> Syzbot reported a general protection fault:
>
> CPU: 0 UID: 0 PID: 5830 Comm: syz-executor600 Not tainted 6.14.0-rc4-syzkaller-00090-gdd83757f6e68 #0
> RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> RIP: 0010:__smc_diag_dump.constprop.0+0x3de/0x23d0 net/smc/smc_diag.c:89
> Call Trace:
> <TASK>
> smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> smc_diag_dump+0x84/0x90 net/smc/smc_diag.c:236
> netlink_dump+0x53c/0xd00 net/netlink/af_netlink.c:2318
> __netlink_dump_start+0x6ca/0x970 net/netlink/af_netlink.c:2433
> netlink_dump_start include/linux/netlink.h:340 [inline]
> smc_diag_handler_dump+0x1fb/0x240 net/smc/smc_diag.c:251
> __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> sock_diag_rcv_msg+0x437/0x790 net/core/sock_diag.c:287
> netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2543
> netlink_unicast_kernel net/netlink/af_netlink.c:1322 [inline]
> netlink_unicast+0x53c/0x7f0 net/netlink/af_netlink.c:1348
> netlink_sendmsg+0x8b8/0xd70 net/netlink/af_netlink.c:1892
> sock_sendmsg_nosec net/socket.c:718 [inline]
> __sock_sendmsg net/socket.c:733 [inline]
> ____sys_sendmsg+0xaaf/0xc90 net/socket.c:2573
> ___sys_sendmsg+0x135/0x1e0 net/socket.c:2627
> __sys_sendmsg+0x16e/0x220 net/socket.c:2659
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
>
> When create smc socket, smc_inet_init_sock() first add sk to the smc_hash
> by smc_hash_sk(), then create smc->clcsock. it is possible that, after
> smc_diag_dump_proto() traverses the smc_hash, smc->clcsock is not created
> when the function visit it.
>
> The process like this:
>
> (CPU1) | (CPU2)
> inet6_create() |
> smc_inet_init_sock() |
> smc_sk_init() |
> smc_hash_sk() |
> head = &smc_hash->ht; |
> sk_add_node(sk, head); |
> | smc_diag_dump_proto
> | head = &smc_hash->ht;
> | sk_for_each(sk, head)
> | __smc_diag_dump()
> | visit smc->clcsock
> smc_create_clcsk() |
> set smc->clcsock |
>
> Fix this by initialize smc->clcsock to NULL before add sk to smc_hash in
> smc_sk_init().
>
> Reported-by: syzbot+271fed3ed6f24600c364@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
> Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
> Signed-off-by: Wang Liang <wangliang74@huawei.com>
> ---
> net/smc/af_smc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 3e6cb35baf25..454801188514 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
> sk->sk_protocol = protocol;
> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
> + smc->clcsock = NULL;
> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
> INIT_WORK(&smc->connect_work, smc_connect_work);
> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
I have to agree with this workaround, even though I see that is not the
best solution. Thus, I'd like to give my R-b:
Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
Btw. @D. Wythe, would you mind sending me the link of your proposal you
mentioned please? Let me have a look. It seems like I missed it.
Thanks,
Wenjia
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-04-03 11:55 ` Wenjia Zhang
@ 2025-04-10 3:11 ` Wang Liang
0 siblings, 0 replies; 17+ messages in thread
From: Wang Liang @ 2025-04-10 3:11 UTC (permalink / raw)
To: Wenjia Zhang, jaka, alibuda, tonylu, guwen, davem, edumazet, kuba,
pabeni, horms, ubraun, Sidraya Jayagond, Mahanta Jambigi
Cc: yuehaibing, zhangchangzhong, linux-rdma, linux-s390, netdev,
linux-kernel
在 2025/4/3 19:55, Wenjia Zhang 写道:
>
>
> On 31.03.25 10:10, Wang Liang wrote:
>> Syzbot reported a general protection fault:
>>
>> CPU: 0 UID: 0 PID: 5830 Comm: syz-executor600 Not tainted
>> 6.14.0-rc4-syzkaller-00090-gdd83757f6e68 #0
>> RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
>> RIP: 0010:__smc_diag_dump.constprop.0+0x3de/0x23d0
>> net/smc/smc_diag.c:89
>> Call Trace:
>> <TASK>
>> smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
>> smc_diag_dump+0x84/0x90 net/smc/smc_diag.c:236
>> netlink_dump+0x53c/0xd00 net/netlink/af_netlink.c:2318
>> __netlink_dump_start+0x6ca/0x970 net/netlink/af_netlink.c:2433
>> netlink_dump_start include/linux/netlink.h:340 [inline]
>> smc_diag_handler_dump+0x1fb/0x240 net/smc/smc_diag.c:251
>> __sock_diag_cmd net/core/sock_diag.c:249 [inline]
>> sock_diag_rcv_msg+0x437/0x790 net/core/sock_diag.c:287
>> netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2543
>> netlink_unicast_kernel net/netlink/af_netlink.c:1322 [inline]
>> netlink_unicast+0x53c/0x7f0 net/netlink/af_netlink.c:1348
>> netlink_sendmsg+0x8b8/0xd70 net/netlink/af_netlink.c:1892
>> sock_sendmsg_nosec net/socket.c:718 [inline]
>> __sock_sendmsg net/socket.c:733 [inline]
>> ____sys_sendmsg+0xaaf/0xc90 net/socket.c:2573
>> ___sys_sendmsg+0x135/0x1e0 net/socket.c:2627
>> __sys_sendmsg+0x16e/0x220 net/socket.c:2659
>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>> do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>> </TASK>
>>
>> When create smc socket, smc_inet_init_sock() first add sk to the
>> smc_hash
>> by smc_hash_sk(), then create smc->clcsock. it is possible that, after
>> smc_diag_dump_proto() traverses the smc_hash, smc->clcsock is not
>> created
>> when the function visit it.
>>
>> The process like this:
>>
>> (CPU1) | (CPU2)
>> inet6_create() |
>> smc_inet_init_sock() |
>> smc_sk_init() |
>> smc_hash_sk() |
>> head = &smc_hash->ht; |
>> sk_add_node(sk, head); |
>> | smc_diag_dump_proto
>> | head = &smc_hash->ht;
>> | sk_for_each(sk, head)
>> | __smc_diag_dump()
>> | visit smc->clcsock
>> smc_create_clcsk() |
>> set smc->clcsock |
>>
>> Fix this by initialize smc->clcsock to NULL before add sk to smc_hash in
>> smc_sk_init().
>>
>> Reported-by: syzbot+271fed3ed6f24600c364@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=271fed3ed6f24600c364
>> Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>> ---
>> net/smc/af_smc.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index 3e6cb35baf25..454801188514 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -371,6 +371,7 @@ void smc_sk_init(struct net *net, struct sock
>> *sk, int protocol)
>> sk->sk_protocol = protocol;
>> WRITE_ONCE(sk->sk_sndbuf, 2 * READ_ONCE(net->smc.sysctl_wmem));
>> WRITE_ONCE(sk->sk_rcvbuf, 2 * READ_ONCE(net->smc.sysctl_rmem));
>> + smc->clcsock = NULL;
>> INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
>> INIT_WORK(&smc->connect_work, smc_connect_work);
>> INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
>
> I have to agree with this workaround, even though I see that is not
> the best solution. Thus, I'd like to give my R-b:
>
> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
>
> Btw. @D. Wythe, would you mind sending me the link of your proposal
> you mentioned please? Let me have a look. It seems like I missed it.
>
> Thanks,
> Wenjia
>
Hello, is this patch rejected?
If there are some new fix patchs, please let me know.
Thanks.
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
@ 2025-09-22 12:18 Wang Liang
2025-09-25 14:32 ` Eric Dumazet
0 siblings, 1 reply; 17+ messages in thread
From: Wang Liang @ 2025-09-22 12:18 UTC (permalink / raw)
To: alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu, guwen, davem,
edumazet, kuba, pabeni, horms
Cc: yuehaibing, zhangchangzhong, wangliang74, linux-kernel, netdev,
linux-rdma, linux-s390
The syzbot report a crash:
Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
Call Trace:
<TASK>
smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
__netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
netlink_dump_start include/linux/netlink.h:341 [inline]
smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
__sock_diag_cmd net/core/sock_diag.c:249 [inline]
sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
sock_sendmsg_nosec net/socket.c:714 [inline]
__sock_sendmsg net/socket.c:729 [inline]
____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
__sys_sendmsg+0x16d/0x220 net/socket.c:2700
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
The process like this:
(CPU1) | (CPU2)
---------------------------------|-------------------------------
inet_create() |
// init clcsock to NULL |
sk = sk_alloc() |
|
// unexpectedly change clcsock |
inet_init_csk_locks() |
|
// add sk to hash table |
smc_inet_init_sock() |
smc_sk_init() |
smc_hash_sk() |
| // traverse the hash table
| smc_diag_dump_proto
| __smc_diag_dump()
| // visit wrong clcsock
| smc_diag_msg_common_fill()
// alloc clcsock |
smc_create_clcsk |
sock_create_kern |
With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
in inet_init_csk_locks(), because the struct smc_sock does not have struct
inet_connection_sock as the first member.
Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
confusion.") add inet_sock as the first member of smc_sock. For protocol
with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
more appropriate.
Reported-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f775be4458668f7d220e
Tested-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
Signed-off-by: Wang Liang <wangliang74@huawei.com>
---
net/smc/smc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 2c9084963739..1b20f0c927d3 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -285,7 +285,7 @@ struct smc_connection {
struct smc_sock { /* smc sock container */
union {
struct sock sk;
- struct inet_sock icsk_inet;
+ struct inet_connection_sock inet_conn;
};
struct socket *clcsock; /* internal tcp socket */
void (*clcsk_state_change)(struct sock *sk);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-22 12:18 Wang Liang
@ 2025-09-25 14:32 ` Eric Dumazet
2025-09-25 18:46 ` Kuniyuki Iwashima
0 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2025-09-25 14:32 UTC (permalink / raw)
To: Wang Liang, Kuniyuki Iwashima
Cc: alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu, guwen, davem,
kuba, pabeni, horms, yuehaibing, zhangchangzhong, linux-kernel,
netdev, linux-rdma, linux-s390
On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
>
> The syzbot report a crash:
>
> Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> Call Trace:
> <TASK>
> smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> netlink_dump_start include/linux/netlink.h:341 [inline]
> smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> sock_sendmsg_nosec net/socket.c:714 [inline]
> __sock_sendmsg net/socket.c:729 [inline]
> ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
>
> The process like this:
>
> (CPU1) | (CPU2)
> ---------------------------------|-------------------------------
> inet_create() |
> // init clcsock to NULL |
> sk = sk_alloc() |
> |
> // unexpectedly change clcsock |
> inet_init_csk_locks() |
> |
> // add sk to hash table |
> smc_inet_init_sock() |
> smc_sk_init() |
> smc_hash_sk() |
> | // traverse the hash table
> | smc_diag_dump_proto
> | __smc_diag_dump()
> | // visit wrong clcsock
> | smc_diag_msg_common_fill()
> // alloc clcsock |
> smc_create_clcsk |
> sock_create_kern |
>
> With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> in inet_init_csk_locks(), because the struct smc_sock does not have struct
> inet_connection_sock as the first member.
>
> Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> confusion.") add inet_sock as the first member of smc_sock. For protocol
> with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> more appropriate.
>
> Reported-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f775be4458668f7d220e
> Tested-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
> Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
> Signed-off-by: Wang Liang <wangliang74@huawei.com>
> ---
> net/smc/smc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 2c9084963739..1b20f0c927d3 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -285,7 +285,7 @@ struct smc_connection {
> struct smc_sock { /* smc sock container */
> union {
> struct sock sk;
> - struct inet_sock icsk_inet;
> + struct inet_connection_sock inet_conn;
> };
> struct socket *clcsock; /* internal tcp socket */
> void (*clcsk_state_change)(struct sock *sk);
> --
> 2.34.1
>
Kuniyuki, can you please review, I think you had a related fix recently.
Thanks.
commit 60ada4fe644edaa6c2da97364184b0425e8aeaf5
Author: Kuniyuki Iwashima <kuniyu@google.com>
Date: Fri Jul 11 06:07:52 2025 +0000
smc: Fix various oops due to inet_sock type confusion.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 14:32 ` Eric Dumazet
@ 2025-09-25 18:46 ` Kuniyuki Iwashima
2025-09-25 18:53 ` Eric Dumazet
0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-25 18:46 UTC (permalink / raw)
To: Wang Liang
Cc: Eric Dumazet, alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu,
guwen, davem, kuba, pabeni, horms, yuehaibing, zhangchangzhong,
linux-kernel, netdev, linux-rdma, linux-s390
Thanks Eric for CCing me.
On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> >
> > The syzbot report a crash:
> >
> > Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> > CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> > RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> > RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> > Call Trace:
> > <TASK>
> > smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> > smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> > netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> > __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> > netlink_dump_start include/linux/netlink.h:341 [inline]
> > smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> > __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> > sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> > netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> > netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> > netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> > netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> > sock_sendmsg_nosec net/socket.c:714 [inline]
> > __sock_sendmsg net/socket.c:729 [inline]
> > ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> > ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> > __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > </TASK>
> >
> > The process like this:
> >
> > (CPU1) | (CPU2)
> > ---------------------------------|-------------------------------
> > inet_create() |
> > // init clcsock to NULL |
> > sk = sk_alloc() |
> > |
> > // unexpectedly change clcsock |
> > inet_init_csk_locks() |
> > |
> > // add sk to hash table |
> > smc_inet_init_sock() |
> > smc_sk_init() |
> > smc_hash_sk() |
> > | // traverse the hash table
> > | smc_diag_dump_proto
> > | __smc_diag_dump()
> > | // visit wrong clcsock
> > | smc_diag_msg_common_fill()
> > // alloc clcsock |
> > smc_create_clcsk |
> > sock_create_kern |
> >
> > With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> > in inet_init_csk_locks(), because the struct smc_sock does not have struct
> > inet_connection_sock as the first member.
> >
> > Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> > confusion.") add inet_sock as the first member of smc_sock. For protocol
> > with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> > more appropriate.
Why is INET_PROTOSW_ICSK necessary in the first place ?
I don't see a clear reason because smc_clcsock_accept() allocates
a new sock by smc_sock_alloc() and does not use inet_accept().
Or is there any other path where smc_sock is cast to
inet_connection_sock ?
> >
> > Reported-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f775be4458668f7d220e
> > Tested-by: syzbot+f775be4458668f7d220e@syzkaller.appspotmail.com
> > Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
> > Signed-off-by: Wang Liang <wangliang74@huawei.com>
> > ---
> > net/smc/smc.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/smc/smc.h b/net/smc/smc.h
> > index 2c9084963739..1b20f0c927d3 100644
> > --- a/net/smc/smc.h
> > +++ b/net/smc/smc.h
> > @@ -285,7 +285,7 @@ struct smc_connection {
> > struct smc_sock { /* smc sock container */
> > union {
> > struct sock sk;
> > - struct inet_sock icsk_inet;
> > + struct inet_connection_sock inet_conn;
> > };
> > struct socket *clcsock; /* internal tcp socket */
> > void (*clcsk_state_change)(struct sock *sk);
> > --
> > 2.34.1
> >
>
> Kuniyuki, can you please review, I think you had a related fix recently.
>
> Thanks.
>
> commit 60ada4fe644edaa6c2da97364184b0425e8aeaf5
> Author: Kuniyuki Iwashima <kuniyu@google.com>
> Date: Fri Jul 11 06:07:52 2025 +0000
>
> smc: Fix various oops due to inet_sock type confusion.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 18:46 ` Kuniyuki Iwashima
@ 2025-09-25 18:53 ` Eric Dumazet
2025-09-25 19:25 ` Kuniyuki Iwashima
0 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2025-09-25 18:53 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Wang Liang, alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu,
guwen, davem, kuba, pabeni, horms, yuehaibing, zhangchangzhong,
linux-kernel, netdev, linux-rdma, linux-s390
On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> Thanks Eric for CCing me.
>
> On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> > >
> > > The syzbot report a crash:
> > >
> > > Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> > > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> > > CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> > > RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> > > RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> > > Call Trace:
> > > <TASK>
> > > smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> > > smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> > > netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> > > __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> > > netlink_dump_start include/linux/netlink.h:341 [inline]
> > > smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> > > __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> > > sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> > > netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> > > netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> > > netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> > > netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> > > sock_sendmsg_nosec net/socket.c:714 [inline]
> > > __sock_sendmsg net/socket.c:729 [inline]
> > > ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> > > ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> > > __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > > do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > </TASK>
> > >
> > > The process like this:
> > >
> > > (CPU1) | (CPU2)
> > > ---------------------------------|-------------------------------
> > > inet_create() |
> > > // init clcsock to NULL |
> > > sk = sk_alloc() |
> > > |
> > > // unexpectedly change clcsock |
> > > inet_init_csk_locks() |
> > > |
> > > // add sk to hash table |
> > > smc_inet_init_sock() |
> > > smc_sk_init() |
> > > smc_hash_sk() |
> > > | // traverse the hash table
> > > | smc_diag_dump_proto
> > > | __smc_diag_dump()
> > > | // visit wrong clcsock
> > > | smc_diag_msg_common_fill()
> > > // alloc clcsock |
> > > smc_create_clcsk |
> > > sock_create_kern |
> > >
> > > With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> > > in inet_init_csk_locks(), because the struct smc_sock does not have struct
> > > inet_connection_sock as the first member.
> > >
> > > Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> > > confusion.") add inet_sock as the first member of smc_sock. For protocol
> > > with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> > > more appropriate.
>
> Why is INET_PROTOSW_ICSK necessary in the first place ?
>
> I don't see a clear reason because smc_clcsock_accept() allocates
> a new sock by smc_sock_alloc() and does not use inet_accept().
>
> Or is there any other path where smc_sock is cast to
> inet_connection_sock ?
What I saw in this code was a missing protection.
smc_diag_msg_common_fill() runs without socket lock being held.
I was thinking of this fix, but apparently syzbot still got crashes.
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 10219f55aad14d795dabe4331458bd1b73c22789..b6abd0efea22c0c9726090b5de60e648b86e09a0
100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -30,7 +30,8 @@ void smc_clcsock_release(struct smc_sock *smc)
mutex_lock(&smc->clcsock_release_lock);
if (smc->clcsock) {
tcp = smc->clcsock;
- smc->clcsock = NULL;
+ WRITE_ONCE(smc->clcsock, NULL);
+ synchronize_rcu();
sock_release(tcp);
}
mutex_unlock(&smc->clcsock_release_lock);
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index bf0beaa23bdb63edfe0c37515aa17a04bb648c08..069607c1db9aff76d1d4f23b47dfeb5177c433d8
100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -35,26 +35,32 @@ static struct smc_diag_dump_ctx
*smc_dump_context(struct netlink_callback *cb)
static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
{
struct smc_sock *smc = smc_sk(sk);
+ struct socket *clcsock;
memset(r, 0, sizeof(*r));
r->diag_family = sk->sk_family;
sock_diag_save_cookie(sk, r->id.idiag_cookie);
- if (!smc->clcsock)
- return;
- r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
- r->id.idiag_dport = smc->clcsock->sk->sk_dport;
- r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
+
+ rcu_read_lock();
+ clcsock = READ_ONCE(smc->clcsock);
+ if (!clcsock)
+ goto unlock;
+ r->id.idiag_sport = htons(clcsock->sk->sk_num);
+ r->id.idiag_dport = clcsock->sk->sk_dport;
+ r->id.idiag_if = clcsock->sk->sk_bound_dev_if;
if (sk->sk_protocol == SMCPROTO_SMC) {
- r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
- r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
+ r->id.idiag_src[0] = clcsock->sk->sk_rcv_saddr;
+ r->id.idiag_dst[0] = clcsock->sk->sk_daddr;
#if IS_ENABLED(CONFIG_IPV6)
} else if (sk->sk_protocol == SMCPROTO_SMC6) {
- memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
- sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
- memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
- sizeof(smc->clcsock->sk->sk_v6_daddr));
+ memcpy(&r->id.idiag_src, &clcsock->sk->sk_v6_rcv_saddr,
+ sizeof(clcsock->sk->sk_v6_rcv_saddr));
+ memcpy(&r->id.idiag_dst, &clcsock->sk->sk_v6_daddr,
+ sizeof(clcsock->sk->sk_v6_daddr));
#endif
}
+unlock:
+ rcu_read_unlock();
}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 18:53 ` Eric Dumazet
@ 2025-09-25 19:25 ` Kuniyuki Iwashima
2025-09-25 19:37 ` Eric Dumazet
0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-25 19:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Wang Liang, alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu,
guwen, davem, kuba, pabeni, horms, yuehaibing, zhangchangzhong,
linux-kernel, netdev, linux-rdma, linux-s390
On Thu, Sep 25, 2025 at 11:54 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > Thanks Eric for CCing me.
> >
> > On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> > > >
> > > > The syzbot report a crash:
> > > >
> > > > Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> > > > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> > > > CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> > > > RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> > > > RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> > > > Call Trace:
> > > > <TASK>
> > > > smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> > > > smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> > > > netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> > > > __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> > > > netlink_dump_start include/linux/netlink.h:341 [inline]
> > > > smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> > > > __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> > > > sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> > > > netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> > > > netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> > > > netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> > > > netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> > > > sock_sendmsg_nosec net/socket.c:714 [inline]
> > > > __sock_sendmsg net/socket.c:729 [inline]
> > > > ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> > > > ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> > > > __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> > > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > > > do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > > </TASK>
> > > >
> > > > The process like this:
> > > >
> > > > (CPU1) | (CPU2)
> > > > ---------------------------------|-------------------------------
> > > > inet_create() |
> > > > // init clcsock to NULL |
> > > > sk = sk_alloc() |
> > > > |
> > > > // unexpectedly change clcsock |
> > > > inet_init_csk_locks() |
> > > > |
> > > > // add sk to hash table |
> > > > smc_inet_init_sock() |
> > > > smc_sk_init() |
> > > > smc_hash_sk() |
> > > > | // traverse the hash table
> > > > | smc_diag_dump_proto
> > > > | __smc_diag_dump()
> > > > | // visit wrong clcsock
> > > > | smc_diag_msg_common_fill()
> > > > // alloc clcsock |
> > > > smc_create_clcsk |
> > > > sock_create_kern |
> > > >
> > > > With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> > > > in inet_init_csk_locks(), because the struct smc_sock does not have struct
> > > > inet_connection_sock as the first member.
> > > >
> > > > Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> > > > confusion.") add inet_sock as the first member of smc_sock. For protocol
> > > > with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> > > > more appropriate.
> >
> > Why is INET_PROTOSW_ICSK necessary in the first place ?
> >
> > I don't see a clear reason because smc_clcsock_accept() allocates
> > a new sock by smc_sock_alloc() and does not use inet_accept().
> >
> > Or is there any other path where smc_sock is cast to
> > inet_connection_sock ?
>
> What I saw in this code was a missing protection.
>
> smc_diag_msg_common_fill() runs without socket lock being held.
>
> I was thinking of this fix, but apparently syzbot still got crashes.
Looking at the test result,
https://syzkaller.appspot.com/x/report.txt?x=15944c7c580000
KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
the top half of the address is SPINLOCK_MAGIC (0xdead4ead),
so the type confusion mentioned in the commit message makes
sense to me.
$ pahole -C inet_connection_sock vmlinux
struct inet_connection_sock {
...
struct request_sock_queue icsk_accept_queue; /* 992 80 */
$ pahole -C smc_sock vmlinux
struct smc_sock {
...
struct socket * clcsock; /* 992 8 */
The option is 1) let inet_init_csk_locks() init inet_connection_sock
or 2) avoid inet_init_csk_locks(), and I guess 2) could be better to
avoid potential issues in IS_ICSK branches.
>
> diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
> index 10219f55aad14d795dabe4331458bd1b73c22789..b6abd0efea22c0c9726090b5de60e648b86e09a0
> 100644
> --- a/net/smc/smc_close.c
> +++ b/net/smc/smc_close.c
> @@ -30,7 +30,8 @@ void smc_clcsock_release(struct smc_sock *smc)
> mutex_lock(&smc->clcsock_release_lock);
> if (smc->clcsock) {
> tcp = smc->clcsock;
> - smc->clcsock = NULL;
> + WRITE_ONCE(smc->clcsock, NULL);
> + synchronize_rcu();
> sock_release(tcp);
> }
> mutex_unlock(&smc->clcsock_release_lock);
> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
> index bf0beaa23bdb63edfe0c37515aa17a04bb648c08..069607c1db9aff76d1d4f23b47dfeb5177c433d8
> 100644
> --- a/net/smc/smc_diag.c
> +++ b/net/smc/smc_diag.c
> @@ -35,26 +35,32 @@ static struct smc_diag_dump_ctx
> *smc_dump_context(struct netlink_callback *cb)
> static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
> {
> struct smc_sock *smc = smc_sk(sk);
> + struct socket *clcsock;
>
> memset(r, 0, sizeof(*r));
> r->diag_family = sk->sk_family;
> sock_diag_save_cookie(sk, r->id.idiag_cookie);
> - if (!smc->clcsock)
> - return;
> - r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
> - r->id.idiag_dport = smc->clcsock->sk->sk_dport;
> - r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
> +
> + rcu_read_lock();
> + clcsock = READ_ONCE(smc->clcsock);
> + if (!clcsock)
> + goto unlock;
> + r->id.idiag_sport = htons(clcsock->sk->sk_num);
> + r->id.idiag_dport = clcsock->sk->sk_dport;
> + r->id.idiag_if = clcsock->sk->sk_bound_dev_if;
> if (sk->sk_protocol == SMCPROTO_SMC) {
> - r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
> - r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
> + r->id.idiag_src[0] = clcsock->sk->sk_rcv_saddr;
> + r->id.idiag_dst[0] = clcsock->sk->sk_daddr;
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (sk->sk_protocol == SMCPROTO_SMC6) {
> - memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
> - sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
> - memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
> - sizeof(smc->clcsock->sk->sk_v6_daddr));
> + memcpy(&r->id.idiag_src, &clcsock->sk->sk_v6_rcv_saddr,
> + sizeof(clcsock->sk->sk_v6_rcv_saddr));
> + memcpy(&r->id.idiag_dst, &clcsock->sk->sk_v6_daddr,
> + sizeof(clcsock->sk->sk_v6_daddr));
> #endif
> }
> +unlock:
> + rcu_read_unlock();
> }
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 19:25 ` Kuniyuki Iwashima
@ 2025-09-25 19:37 ` Eric Dumazet
2025-09-25 19:51 ` Kuniyuki Iwashima
0 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2025-09-25 19:37 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Wang Liang, alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu,
guwen, davem, kuba, pabeni, horms, yuehaibing, zhangchangzhong,
linux-kernel, netdev, linux-rdma, linux-s390
On Thu, Sep 25, 2025 at 12:25 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Thu, Sep 25, 2025 at 11:54 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> > >
> > > Thanks Eric for CCing me.
> > >
> > > On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > > On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> > > > >
> > > > > The syzbot report a crash:
> > > > >
> > > > > Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> > > > > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> > > > > CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> > > > > RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> > > > > RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> > > > > Call Trace:
> > > > > <TASK>
> > > > > smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> > > > > smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> > > > > netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> > > > > __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> > > > > netlink_dump_start include/linux/netlink.h:341 [inline]
> > > > > smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> > > > > __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> > > > > sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> > > > > netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> > > > > netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> > > > > netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> > > > > netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> > > > > sock_sendmsg_nosec net/socket.c:714 [inline]
> > > > > __sock_sendmsg net/socket.c:729 [inline]
> > > > > ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> > > > > ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> > > > > __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> > > > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > > > > do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> > > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > > > </TASK>
> > > > >
> > > > > The process like this:
> > > > >
> > > > > (CPU1) | (CPU2)
> > > > > ---------------------------------|-------------------------------
> > > > > inet_create() |
> > > > > // init clcsock to NULL |
> > > > > sk = sk_alloc() |
> > > > > |
> > > > > // unexpectedly change clcsock |
> > > > > inet_init_csk_locks() |
> > > > > |
> > > > > // add sk to hash table |
> > > > > smc_inet_init_sock() |
> > > > > smc_sk_init() |
> > > > > smc_hash_sk() |
> > > > > | // traverse the hash table
> > > > > | smc_diag_dump_proto
> > > > > | __smc_diag_dump()
> > > > > | // visit wrong clcsock
> > > > > | smc_diag_msg_common_fill()
> > > > > // alloc clcsock |
> > > > > smc_create_clcsk |
> > > > > sock_create_kern |
> > > > >
> > > > > With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> > > > > in inet_init_csk_locks(), because the struct smc_sock does not have struct
> > > > > inet_connection_sock as the first member.
> > > > >
> > > > > Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> > > > > confusion.") add inet_sock as the first member of smc_sock. For protocol
> > > > > with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> > > > > more appropriate.
> > >
> > > Why is INET_PROTOSW_ICSK necessary in the first place ?
> > >
> > > I don't see a clear reason because smc_clcsock_accept() allocates
> > > a new sock by smc_sock_alloc() and does not use inet_accept().
> > >
> > > Or is there any other path where smc_sock is cast to
> > > inet_connection_sock ?
> >
> > What I saw in this code was a missing protection.
> >
> > smc_diag_msg_common_fill() runs without socket lock being held.
> >
> > I was thinking of this fix, but apparently syzbot still got crashes.
>
> Looking at the test result,
>
> https://syzkaller.appspot.com/x/report.txt?x=15944c7c580000
> KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
>
> the top half of the address is SPINLOCK_MAGIC (0xdead4ead),
> so the type confusion mentioned in the commit message makes
> sense to me.
>
> $ pahole -C inet_connection_sock vmlinux
> struct inet_connection_sock {
> ...
> struct request_sock_queue icsk_accept_queue; /* 992 80 */
>
> $ pahole -C smc_sock vmlinux
> struct smc_sock {
> ...
> struct socket * clcsock; /* 992 8 */
>
> The option is 1) let inet_init_csk_locks() init inet_connection_sock
> or 2) avoid inet_init_csk_locks(), and I guess 2) could be better to
> avoid potential issues in IS_ICSK branches.
>
I definitely vote to remove INET_PROTOSW_ICSK from smc.
We want to reserve inet_connection_sock to TCP only, so that we can
move fields to better
cache friendly locations in tcp_sock hopefully for linux-6.19
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 19:37 ` Eric Dumazet
@ 2025-09-25 19:51 ` Kuniyuki Iwashima
2025-09-26 8:42 ` Wang Liang
0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-25 19:51 UTC (permalink / raw)
To: Eric Dumazet, Wang Liang
Cc: alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu, guwen, davem,
kuba, pabeni, horms, yuehaibing, zhangchangzhong, linux-kernel,
netdev, linux-rdma, linux-s390
On Thu, Sep 25, 2025 at 12:37 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Sep 25, 2025 at 12:25 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > On Thu, Sep 25, 2025 at 11:54 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> > > >
> > > > Thanks Eric for CCing me.
> > > >
> > > > On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
> > > > >
> > > > > On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> > > > > >
> > > > > > The syzbot report a crash:
> > > > > >
> > > > > > Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> > > > > > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> > > > > > CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> > > > > > RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> > > > > > RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> > > > > > Call Trace:
> > > > > > <TASK>
> > > > > > smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> > > > > > smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> > > > > > netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> > > > > > __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> > > > > > netlink_dump_start include/linux/netlink.h:341 [inline]
> > > > > > smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> > > > > > __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> > > > > > sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> > > > > > netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> > > > > > netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> > > > > > netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> > > > > > netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> > > > > > sock_sendmsg_nosec net/socket.c:714 [inline]
> > > > > > __sock_sendmsg net/socket.c:729 [inline]
> > > > > > ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> > > > > > ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> > > > > > __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> > > > > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > > > > > do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> > > > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > > > > </TASK>
> > > > > >
> > > > > > The process like this:
> > > > > >
> > > > > > (CPU1) | (CPU2)
> > > > > > ---------------------------------|-------------------------------
> > > > > > inet_create() |
> > > > > > // init clcsock to NULL |
> > > > > > sk = sk_alloc() |
> > > > > > |
> > > > > > // unexpectedly change clcsock |
> > > > > > inet_init_csk_locks() |
> > > > > > |
> > > > > > // add sk to hash table |
> > > > > > smc_inet_init_sock() |
> > > > > > smc_sk_init() |
> > > > > > smc_hash_sk() |
> > > > > > | // traverse the hash table
> > > > > > | smc_diag_dump_proto
> > > > > > | __smc_diag_dump()
> > > > > > | // visit wrong clcsock
> > > > > > | smc_diag_msg_common_fill()
> > > > > > // alloc clcsock |
> > > > > > smc_create_clcsk |
> > > > > > sock_create_kern |
> > > > > >
> > > > > > With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> > > > > > in inet_init_csk_locks(), because the struct smc_sock does not have struct
> > > > > > inet_connection_sock as the first member.
> > > > > >
> > > > > > Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> > > > > > confusion.") add inet_sock as the first member of smc_sock. For protocol
> > > > > > with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> > > > > > more appropriate.
> > > >
> > > > Why is INET_PROTOSW_ICSK necessary in the first place ?
> > > >
> > > > I don't see a clear reason because smc_clcsock_accept() allocates
> > > > a new sock by smc_sock_alloc() and does not use inet_accept().
> > > >
> > > > Or is there any other path where smc_sock is cast to
> > > > inet_connection_sock ?
> > >
> > > What I saw in this code was a missing protection.
> > >
> > > smc_diag_msg_common_fill() runs without socket lock being held.
> > >
> > > I was thinking of this fix, but apparently syzbot still got crashes.
> >
> > Looking at the test result,
> >
> > https://syzkaller.appspot.com/x/report.txt?x=15944c7c580000
> > KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> >
> > the top half of the address is SPINLOCK_MAGIC (0xdead4ead),
> > so the type confusion mentioned in the commit message makes
> > sense to me.
> >
> > $ pahole -C inet_connection_sock vmlinux
> > struct inet_connection_sock {
> > ...
> > struct request_sock_queue icsk_accept_queue; /* 992 80 */
> >
> > $ pahole -C smc_sock vmlinux
> > struct smc_sock {
> > ...
> > struct socket * clcsock; /* 992 8 */
> >
> > The option is 1) let inet_init_csk_locks() init inet_connection_sock
> > or 2) avoid inet_init_csk_locks(), and I guess 2) could be better to
> > avoid potential issues in IS_ICSK branches.
> >
>
> I definitely vote to remove INET_PROTOSW_ICSK from smc.
>
> We want to reserve inet_connection_sock to TCP only, so that we can
> move fields to better
> cache friendly locations in tcp_sock hopefully for linux-6.19
Fully agreed.
Wang: please squash the revert of 6fd27ea183c2 for
INET_PROTOSW_ICSK removal. This is for one of
IS_ICSK branches.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-25 19:51 ` Kuniyuki Iwashima
@ 2025-09-26 8:42 ` Wang Liang
2025-10-14 2:04 ` D. Wythe
0 siblings, 1 reply; 17+ messages in thread
From: Wang Liang @ 2025-09-26 8:42 UTC (permalink / raw)
To: Kuniyuki Iwashima, Eric Dumazet
Cc: alibuda, dust.li, sidraya, wenjia, mjambigi, tonylu, guwen, davem,
kuba, pabeni, horms, yuehaibing, zhangchangzhong, linux-kernel,
netdev, linux-rdma, linux-s390
在 2025/9/26 3:51, Kuniyuki Iwashima 写道:
> On Thu, Sep 25, 2025 at 12:37 PM Eric Dumazet <edumazet@google.com> wrote:
>> On Thu, Sep 25, 2025 at 12:25 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>>> On Thu, Sep 25, 2025 at 11:54 AM Eric Dumazet <edumazet@google.com> wrote:
>>>> On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>>>>> Thanks Eric for CCing me.
>>>>>
>>>>> On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
>>>>>> On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
>>>>>>> The syzbot report a crash:
>>>>>>>
>>>>>>> Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
>>>>>>> KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
>>>>>>> CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
>>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
>>>>>>> RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
>>>>>>> RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
>>>>>>> Call Trace:
>>>>>>> <TASK>
>>>>>>> smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
>>>>>>> smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
>>>>>>> netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
>>>>>>> __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
>>>>>>> netlink_dump_start include/linux/netlink.h:341 [inline]
>>>>>>> smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
>>>>>>> __sock_diag_cmd net/core/sock_diag.c:249 [inline]
>>>>>>> sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
>>>>>>> netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
>>>>>>> netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
>>>>>>> netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
>>>>>>> netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
>>>>>>> sock_sendmsg_nosec net/socket.c:714 [inline]
>>>>>>> __sock_sendmsg net/socket.c:729 [inline]
>>>>>>> ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
>>>>>>> ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
>>>>>>> __sys_sendmsg+0x16d/0x220 net/socket.c:2700
>>>>>>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>>>>>>> do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
>>>>>>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>>>>>> </TASK>
>>>>>>>
>>>>>>> The process like this:
>>>>>>>
>>>>>>> (CPU1) | (CPU2)
>>>>>>> ---------------------------------|-------------------------------
>>>>>>> inet_create() |
>>>>>>> // init clcsock to NULL |
>>>>>>> sk = sk_alloc() |
>>>>>>> |
>>>>>>> // unexpectedly change clcsock |
>>>>>>> inet_init_csk_locks() |
>>>>>>> |
>>>>>>> // add sk to hash table |
>>>>>>> smc_inet_init_sock() |
>>>>>>> smc_sk_init() |
>>>>>>> smc_hash_sk() |
>>>>>>> | // traverse the hash table
>>>>>>> | smc_diag_dump_proto
>>>>>>> | __smc_diag_dump()
>>>>>>> | // visit wrong clcsock
>>>>>>> | smc_diag_msg_common_fill()
>>>>>>> // alloc clcsock |
>>>>>>> smc_create_clcsk |
>>>>>>> sock_create_kern |
>>>>>>>
>>>>>>> With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
>>>>>>> in inet_init_csk_locks(), because the struct smc_sock does not have struct
>>>>>>> inet_connection_sock as the first member.
>>>>>>>
>>>>>>> Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
>>>>>>> confusion.") add inet_sock as the first member of smc_sock. For protocol
>>>>>>> with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
>>>>>>> more appropriate.
>>>>> Why is INET_PROTOSW_ICSK necessary in the first place ?
>>>>>
>>>>> I don't see a clear reason because smc_clcsock_accept() allocates
>>>>> a new sock by smc_sock_alloc() and does not use inet_accept().
>>>>>
>>>>> Or is there any other path where smc_sock is cast to
>>>>> inet_connection_sock ?
>>>> What I saw in this code was a missing protection.
>>>>
>>>> smc_diag_msg_common_fill() runs without socket lock being held.
>>>>
>>>> I was thinking of this fix, but apparently syzbot still got crashes.
>>> Looking at the test result,
>>>
>>> https://syzkaller.appspot.com/x/report.txt?x=15944c7c580000
>>> KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
>>>
>>> the top half of the address is SPINLOCK_MAGIC (0xdead4ead),
>>> so the type confusion mentioned in the commit message makes
>>> sense to me.
>>>
>>> $ pahole -C inet_connection_sock vmlinux
>>> struct inet_connection_sock {
>>> ...
>>> struct request_sock_queue icsk_accept_queue; /* 992 80 */
>>>
>>> $ pahole -C smc_sock vmlinux
>>> struct smc_sock {
>>> ...
>>> struct socket * clcsock; /* 992 8 */
>>>
>>> The option is 1) let inet_init_csk_locks() init inet_connection_sock
>>> or 2) avoid inet_init_csk_locks(), and I guess 2) could be better to
>>> avoid potential issues in IS_ICSK branches.
>>>
>> I definitely vote to remove INET_PROTOSW_ICSK from smc.
>>
>> We want to reserve inet_connection_sock to TCP only, so that we can
>> move fields to better
>> cache friendly locations in tcp_sock hopefully for linux-6.19
> Fully agreed.
>
> Wang: please squash the revert of 6fd27ea183c2 for
> INET_PROTOSW_ICSK removal. This is for one of
> IS_ICSK branches.
Thanks for your suggestions, they are helpful!
I will remove INET_PROTOSW_ICSK from smc_inet_protosw and smc_inet6_protosw,
and revert 6fd27ea183c2 ("net/smc: fix lacks of icsk_syn_mss with
IPPROTO_SMC")
in one patchset later.
------
Best regards
Wang Liang
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net] net/smc: fix general protection fault in __smc_diag_dump
2025-09-26 8:42 ` Wang Liang
@ 2025-10-14 2:04 ` D. Wythe
0 siblings, 0 replies; 17+ messages in thread
From: D. Wythe @ 2025-10-14 2:04 UTC (permalink / raw)
To: Wang Liang
Cc: Kuniyuki Iwashima, Eric Dumazet, alibuda, dust.li, sidraya,
wenjia, mjambigi, tonylu, guwen, davem, kuba, pabeni, horms,
yuehaibing, zhangchangzhong, linux-kernel, netdev, linux-rdma,
linux-s390
On Fri, Sep 26, 2025 at 04:42:35PM +0800, Wang Liang wrote:
>
> 在 2025/9/26 3:51, Kuniyuki Iwashima 写道:
> >On Thu, Sep 25, 2025 at 12:37 PM Eric Dumazet <edumazet@google.com> wrote:
> >>On Thu, Sep 25, 2025 at 12:25 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >>>On Thu, Sep 25, 2025 at 11:54 AM Eric Dumazet <edumazet@google.com> wrote:
> >>>>On Thu, Sep 25, 2025 at 11:46 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >>>>>Thanks Eric for CCing me.
> >>>>>
> >>>>>On Thu, Sep 25, 2025 at 7:32 AM Eric Dumazet <edumazet@google.com> wrote:
> >>>>>>On Mon, Sep 22, 2025 at 4:57 AM Wang Liang <wangliang74@huawei.com> wrote:
> >>>>>>>The syzbot report a crash:
> >>>>>>>
> >>>>>>> Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000003: 0000 [#1] SMP KASAN NOPTI
> >>>>>>> KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> >>>>>>> CPU: 1 UID: 0 PID: 6949 Comm: syz.0.335 Not tainted syzkaller #0 PREEMPT(full)
> >>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
> >>>>>>> RIP: 0010:smc_diag_msg_common_fill net/smc/smc_diag.c:44 [inline]
> >>>>>>> RIP: 0010:__smc_diag_dump.constprop.0+0x3ca/0x2550 net/smc/smc_diag.c:89
> >>>>>>> Call Trace:
> >>>>>>> <TASK>
> >>>>>>> smc_diag_dump_proto+0x26d/0x420 net/smc/smc_diag.c:217
> >>>>>>> smc_diag_dump+0x27/0x90 net/smc/smc_diag.c:234
> >>>>>>> netlink_dump+0x539/0xd30 net/netlink/af_netlink.c:2327
> >>>>>>> __netlink_dump_start+0x6d6/0x990 net/netlink/af_netlink.c:2442
> >>>>>>> netlink_dump_start include/linux/netlink.h:341 [inline]
> >>>>>>> smc_diag_handler_dump+0x1f9/0x240 net/smc/smc_diag.c:251
> >>>>>>> __sock_diag_cmd net/core/sock_diag.c:249 [inline]
> >>>>>>> sock_diag_rcv_msg+0x438/0x790 net/core/sock_diag.c:285
> >>>>>>> netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> >>>>>>> netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> >>>>>>> netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
> >>>>>>> netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
> >>>>>>> sock_sendmsg_nosec net/socket.c:714 [inline]
> >>>>>>> __sock_sendmsg net/socket.c:729 [inline]
> >>>>>>> ____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
> >>>>>>> ___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
> >>>>>>> __sys_sendmsg+0x16d/0x220 net/socket.c:2700
> >>>>>>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> >>>>>>> do_syscall_64+0xcd/0x4e0 arch/x86/entry/syscall_64.c:94
> >>>>>>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >>>>>>> </TASK>
> >>>>>>>
> >>>>>>>The process like this:
> >>>>>>>
> >>>>>>> (CPU1) | (CPU2)
> >>>>>>> ---------------------------------|-------------------------------
> >>>>>>> inet_create() |
> >>>>>>> // init clcsock to NULL |
> >>>>>>> sk = sk_alloc() |
> >>>>>>> |
> >>>>>>> // unexpectedly change clcsock |
> >>>>>>> inet_init_csk_locks() |
> >>>>>>> |
> >>>>>>> // add sk to hash table |
> >>>>>>> smc_inet_init_sock() |
> >>>>>>> smc_sk_init() |
> >>>>>>> smc_hash_sk() |
> >>>>>>> | // traverse the hash table
> >>>>>>> | smc_diag_dump_proto
> >>>>>>> | __smc_diag_dump()
> >>>>>>> | // visit wrong clcsock
> >>>>>>> | smc_diag_msg_common_fill()
> >>>>>>> // alloc clcsock |
> >>>>>>> smc_create_clcsk |
> >>>>>>> sock_create_kern |
> >>>>>>>
> >>>>>>>With CONFIG_DEBUG_LOCK_ALLOC=y, the smc->clcsock is unexpectedly changed
> >>>>>>>in inet_init_csk_locks(), because the struct smc_sock does not have struct
> >>>>>>>inet_connection_sock as the first member.
> >>>>>>>
> >>>>>>>Previous commit 60ada4fe644e ("smc: Fix various oops due to inet_sock type
> >>>>>>>confusion.") add inet_sock as the first member of smc_sock. For protocol
> >>>>>>>with INET_PROTOSW_ICSK, use inet_connection_sock instead of inet_sock is
> >>>>>>>more appropriate.
> >>>>>Why is INET_PROTOSW_ICSK necessary in the first place ?
> >>>>>
> >>>>>I don't see a clear reason because smc_clcsock_accept() allocates
> >>>>>a new sock by smc_sock_alloc() and does not use inet_accept().
> >>>>>
> >>>>>Or is there any other path where smc_sock is cast to
> >>>>>inet_connection_sock ?
> >>>>What I saw in this code was a missing protection.
> >>>>
> >>>>smc_diag_msg_common_fill() runs without socket lock being held.
> >>>>
> >>>>I was thinking of this fix, but apparently syzbot still got crashes.
> >>>Looking at the test result,
> >>>
> >>>https://syzkaller.appspot.com/x/report.txt?x=15944c7c580000
> >>>KASAN: maybe wild-memory-access in range [0xdead4ead00000018-0xdead4ead0000001f]
> >>>
> >>>the top half of the address is SPINLOCK_MAGIC (0xdead4ead),
> >>>so the type confusion mentioned in the commit message makes
> >>>sense to me.
> >>>
> >>>$ pahole -C inet_connection_sock vmlinux
> >>>struct inet_connection_sock {
> >>>...
> >>> struct request_sock_queue icsk_accept_queue; /* 992 80 */
> >>>
> >>>$ pahole -C smc_sock vmlinux
> >>>struct smc_sock {
> >>>...
> >>> struct socket * clcsock; /* 992 8 */
> >>>
> >>>The option is 1) let inet_init_csk_locks() init inet_connection_sock
> >>>or 2) avoid inet_init_csk_locks(), and I guess 2) could be better to
> >>>avoid potential issues in IS_ICSK branches.
> >>>
> >>I definitely vote to remove INET_PROTOSW_ICSK from smc.
> >>
> >>We want to reserve inet_connection_sock to TCP only, so that we can
> >>move fields to better
> >>cache friendly locations in tcp_sock hopefully for linux-6.19
> >Fully agreed.
> >
> >Wang: please squash the revert of 6fd27ea183c2 for
> >INET_PROTOSW_ICSK removal. This is for one of
> >IS_ICSK branches.
>
>
> Thanks for your suggestions, they are helpful!
>
> I will remove INET_PROTOSW_ICSK from smc_inet_protosw and smc_inet6_protosw,
>
This is a bit of a long story. The INET_PROTOSW_ICSK flag was originally
introduced for an effort to merge smc_sock and clcsock. The goal was to
allow the merged socket to behave like a standard tcp_sock during
fallback, thus avoiding the need for any special handling.
However, this approach was later abandoned. Since the original reason
for this flag no longer exists, so the removal makes sense.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-10-14 2:04 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 8:10 [PATCH net] net/smc: fix general protection fault in __smc_diag_dump Wang Liang
2025-04-01 11:01 ` Paolo Abeni
2025-04-01 13:31 ` Zhu Yanjun
2025-04-02 2:37 ` Wang Liang
2025-04-02 7:20 ` D. Wythe
2025-04-03 7:09 ` Wang Liang
2025-04-03 11:55 ` Wenjia Zhang
2025-04-10 3:11 ` Wang Liang
-- strict thread matches above, loose matches on Subject: below --
2025-09-22 12:18 Wang Liang
2025-09-25 14:32 ` Eric Dumazet
2025-09-25 18:46 ` Kuniyuki Iwashima
2025-09-25 18:53 ` Eric Dumazet
2025-09-25 19:25 ` Kuniyuki Iwashima
2025-09-25 19:37 ` Eric Dumazet
2025-09-25 19:51 ` Kuniyuki Iwashima
2025-09-26 8:42 ` Wang Liang
2025-10-14 2:04 ` D. Wythe
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).