Hi Marcelo,
The kernel crashed after applying your patch, and I see SCTP Call Trace in the kernel log.
I double checked the patch fix, and the below logic looks wrong.
@@ -96,13 +97,14 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt, {
int ret;
- if (incnt <= stream->incnt)
- return 0;
+ if (incnt > stream->incnt)
+ goto out;
ret = genradix_prealloc(&stream->in, incnt, gfp);
if (ret)
return ret;
+out:
stream->incnt = incnt;
return 0;
}
Should change to :
@@ -96,13 +97,14 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt, {
int ret;
if (incnt <= stream->incnt)
- return 0;
+ goto out;
ret = genradix_prealloc(&stream->in, incnt, gfp);
if (ret)
return ret;
+out:
stream->incnt = incnt;
return 0;
}
FYI, kernel log.
[ 3064.106545] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 3064.108031] #PF: supervisor read access in kernel mode
[ 3064.108819] #PF: error_code(0x0000) - not-present page
[ 3064.109574] PGD 1b5873067 P4D 1b5873067 PUD 1b5870067 PMD 0
[ 3064.110486] Oops: 0000 [#1] SMP NOPTI
[ 3064.111070] CPU: 3 PID: 0 Comm: swapper/3 Kdump: loaded Not tainted 5.5.4-1.el7.x86_64 #1
[ 3064.112344] Hardware name: Red Hat OpenStack Compute, BIOS 1.10.2-3.el7_4.2 04/01/2014
[ 3064.113551] RIP: 0010:sctp_validate_data+0x75/0x90 [sctp]
[ 3064.114447] Code: c3 08 0f b7 c9 48 89 c8 48 f7 e2 48 c1 ea 07 48 69 c2 cc 00 00 00 48 c1 e2 0c 48 29 c1 48 8d 04 89 48 8d 34 82 e8 eb d3 a3 e0 <66> 2b 18 89 d8 f7 d0 5b 66 c1 e8 0f 5d c3 0f 1f
00 66 2e 0f 1f 84
[ 3064.117321] RSP: 0018:ffffc9000010c6f0 EFLAGS: 00010246
[ 3064.118154] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 3064.119274] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffff
[ 3064.120448] RBP: ffffc9000010c6f8 R08: 000000000000000c R09: 0000000000020000
[ 3064.121485] R10: ffffffffa0a133b0 R11: 00000000beb9068f R12: ffffc9000010c7f0
[ 3064.122542] R13: ffff8881e9b65872 R14: ffff8881da37bf00 R15: 0000000086e372c4
[ 3064.123595] FS: 0000000000000000(0000) GS:ffff888237d80000(0000) knlGS:0000000000000000
[ 3064.124795] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3064.125659] CR2: 0000000000000000 CR3: 00000001b59a4005 CR4: 00000000001606e0
[ 3064.126725] Call Trace:
[ 3064.127094] <IRQ>
[ 3064.127455] sctp_eat_data+0x448/0x5f0 [sctp]
[ 3064.128158] sctp_sf_eat_data_6_2+0xe8/0x2a0 [sctp]
[ 3064.128964] sctp_do_sm+0xa6/0x320 [sctp]
[ 3064.129608] ? sctp_has_association+0x40/0x40 [sctp]
[ 3064.130369] ? xfrm_policy_lookup_bytype+0x165/0x350
[ 3064.131163] ? selinux_xfrm_decode_session+0x13/0x20
[ 3064.131980] ? selinux_peerlbl_enabled+0x1d/0x40
[ 3064.132744] ? selinux_socket_sock_rcv_skb+0x8a/0x230
[ 3064.133567] ? __xfrm_policy_check+0x542/0x840
[ 3064.134240] sctp_assoc_bh_rcv+0x10a/0x1d0 [sctp]
[ 3064.135036] sctp_inq_push+0x6a/0x80 [sctp]
[ 3064.135691] sctp_rcv+0x4f0/0xcc0 [sctp]
[ 3064.136983] ? ip_vs_in+0x4b/0xa0 [ip_vs]
[ 3064.138059] ip_protocol_deliver_rcu+0x195/0x1b0
[ 3064.139262] ip_local_deliver_finish+0x48/0x50
[ 3064.140375] ip_local_deliver+0x6f/0xf0
[ 3064.141463] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
[ 3064.142597] ip_sublist_rcv_finish+0x59/0x70
[ 3064.143737] ip_sublist_rcv+0x187/0x210
[ 3064.144697] ? ip_rcv_core.isra.20+0x290/0x290
[ 3064.145794] ip_list_rcv+0x142/0x170
[ 3064.146761] __netif_receive_skb_list_core+0x2dc/0x310
[ 3064.147945] ? dev_gro_receive+0x683/0x6f0
[ 3064.148974] netif_receive_skb_list_internal+0x1ca/0x2e0
[ 3064.150132] gro_normal_list.part.137+0x1e/0x40
[ 3064.151160] napi_complete_done+0xcd/0x120
[ 3064.152145] virtqueue_napi_complete+0x2f/0x70 [virtio_net]
[ 3064.153349] virtnet_poll+0x318/0x370 [virtio_net]
[ 3064.154466] net_rx_action+0x289/0x400
[ 3064.155399] __do_softirq+0xd9/0x29e
[ 3064.156328] irq_exit+0xcc/0xe0
[ 3064.157195] do_IRQ+0x5a/0xf0
[ 3064.158049] common_interrupt+0xf/0xf
[ 3064.158995] </IRQ>
[ 3064.159619] RIP: 0010:native_safe_halt+0x12/0x20
Thanks,
Jessie and Chris
-----Original Message-----
From: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
Sent: 2020年2月18日 11:40
To: Fan, Jessie (NSB - CN/Qingdao) <jessie.fan@xxxxxxxxxxxxxxx>
Cc: linux-sctp@xxxxxxxxxxxxxxx; dajiang.zhang@xxxxxxxxx; piggy@xxxxxxx; karl@xxxxxxxxxxxxxxxxxxxx; chris@xxxxxxxxxxxxxxxxx; jgrimm@xxxxxxxxxx; xingang.guo@xxxxxxxxx; sri@xxxxxxxxxx; daisyc@xxxxxxxxxx; ardelle.fan@xxxxxxxxx; kevin.gao@xxxxxxxxx; Chen, Chris A. (NSB - CN/Qingdao) <chris.a.chen@xxxxxxxxxxxxxxx>
Subject: Re: v5.3.12 SCTP Stream Negotiation Problem
Hi,
On Tue, Feb 18, 2020 at 12:37:17AM +0000, Fan, Jessie (NSB - CN/Qingdao) wrote:
> Hi,
>
> I found the SCTP Stream negotiation doesn't work as expected, that is, the local outbound stream and the remote inbound stream comparison seems missing.
> For example, when the local outstream(16) is greater than the remote inbound stream(2), 16 is saved and used as the "OUTS", which is shown from /proc/pid/net/sctp/assocs below.
> Can anyone help comment?
>
> From local end point, 16 is set as both the outbound and inbound stream.
> From the remote end point, 2 is set for both the outbound and inbound
> stream
>
> However, after the association is up, the inbound and outbound stream is set as (2,16), which I think is unexpected.
> sh-4.2# cat 1/net/sctp/assocs
> ASSOC SOCK STY SST ST HBKT ASSOC-ID TX_QUEUE RX_QUEUE UID INODE LPORT
> RPORT LADDRS <-> RADDRS HBINT INS OUTS MAXRT T1X T2X RTXC wmema wmemq
> sndbuf rcvbuf 55dae5bb bb3dec72 0 7 3 0 2415 0 0 504 1223451 2905 3904
> xx.xx.xx.xx yy.yy.yy.yy <-> *zz.zz.zz.zz ww.ww.ww.ww 30000 2 16 10 0 0
> 0 1 0 262144 262144
>
> I further checked the kernel code and found in v5.0, there is still comparison logic: min(outcnt, stream->outcnt) and save the smaller one in the function sctp_stream_alloc_out().
> But the logic disappeared in v5.1, and if the "outcnt" is smaller, it's not saved locally.
> static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
> gfp_t gfp) {
> int ret;
>
> if (outcnt <= stream->outcnt) //Here is problematic, and the outcnt is not saved locally.
Makes sense.
Before 2075e50caf5e ("sctp: convert to genradix") it was updating
stream->outcnt with smaller values when applicable:
- if (outcnt > stream->outcnt)
- fa_zero(out, stream->outcnt, (outcnt - stream->outcnt));
+ if (outcnt <= stream->outcnt)
+ return 0;
- stream->out = out;
It also affects the input stream AFAICT.
Can you please try the following patch?
Thanks,
Marcelo
---8<---
diff --git a/net/sctp/stream.c b/net/sctp/stream.c index 67f7e71f9129..34f0b7312fe8 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -81,12 +81,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
int ret;
if (outcnt <= stream->outcnt)
- return 0;
+ goto out;
ret = genradix_prealloc(&stream->out, outcnt, gfp);
if (ret)
return ret;
+out:
stream->outcnt = outcnt;
return 0;
}
@@ -96,13 +97,14 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt, {
int ret;
- if (incnt <= stream->incnt)
- return 0;
+ if (incnt > stream->incnt)
+ goto out;
ret = genradix_prealloc(&stream->in, incnt, gfp);
if (ret)
return ret;
+out:
stream->incnt = incnt;
return 0;
}
