2019-10-30 08:11:20

by Wei Zhao

[permalink] [raw]
Subject: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering

Unlike tcp_transmit_skb,
sctp_packet_transmit does not set ooo_okay explicitly,
causing unwanted Tx queue switching when multiqueue is in use;
Tx queue switching may cause out-of-order packets.
Change sctp_packet_transmit to allow Tx queue switching only for
the first in flight packet, to avoid unwanted Tx queue switching.

Signed-off-by: Wally Zhao <[email protected]>
---
net/sctp/output.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/sctp/output.c b/net/sctp/output.c
index dbda7e7..5ff75cc 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
/* neighbour should be confirmed on successful transmission or
* positive error
*/
+
+ /* allow switch tx queue only for the first in flight pkt */
+ head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;
+
if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
tp->dst_pending_confirm)
tp->dst_pending_confirm = 0;
--
1.8.3.1


2019-10-30 13:27:38

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering

On Wed, Oct 30, 2019 at 12:07:17PM -0400, Wally Zhao wrote:
> Unlike tcp_transmit_skb,
> sctp_packet_transmit does not set ooo_okay explicitly,
> causing unwanted Tx queue switching when multiqueue is in use;

It is initialized to 0 by __alloc_skb() via:
memset(skb, 0, offsetof(struct sk_buff, tail));
and never set to 1 by anyone for SCTP.

The patch description seems off. I don't see how the unwanted Tx queue
switching can happen. IOW, it's not fixing it OOO packets, but
improving it by allowing switching on the first packet. Am I missing
something?

> Tx queue switching may cause out-of-order packets.
> Change sctp_packet_transmit to allow Tx queue switching only for
> the first in flight packet, to avoid unwanted Tx queue switching.
>
> Signed-off-by: Wally Zhao <[email protected]>
> ---
> net/sctp/output.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index dbda7e7..5ff75cc 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> +
> + /* allow switch tx queue only for the first in flight pkt */
> + head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;

Considering we are talking about NIC queues here, we would have a
better result with tp->flight_size instead. As in, we can switch
queues if, for this transport, the queue is empty.

> +
> if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
> --
> 1.8.3.1
>

2019-10-30 16:03:29

by Wei Zhao

[permalink] [raw]
Subject: Re: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering

On 10/30/19, Marcelo Ricardo Leitner <[email protected]> wrote:
> On Wed, Oct 30, 2019 at 12:07:17PM -0400, Wally Zhao wrote:
>> Unlike tcp_transmit_skb,
>> sctp_packet_transmit does not set ooo_okay explicitly,
>> causing unwanted Tx queue switching when multiqueue is in use;
>
> It is initialized to 0 by __alloc_skb() via:
> memset(skb, 0, offsetof(struct sk_buff, tail));
> and never set to 1 by anyone for SCTP.
>
> The patch description seems off. I don't see how the unwanted Tx queue
> switching can happen. IOW, it's not fixing it OOO packets, but
> improving it by allowing switching on the first packet. Am I missing
> something?

Thanks for pointing this out. You are right. This ooo_okay is default to false.

I was observing some Tx queue switching before when testing with
iperf3 (modified to be able to set window size, for higher throughput
with long RTT), so I thought ooo_okay was set to true somewhere else
after allocation. Just now I did the test again, it turns out that
iperf3 made a re-connect silently which caused the Tx queue change.

As for the improving purpose of this patch, that is not that critical
from my side, and the patch description is not correct for this
purpose. So I will give up this patch attempt. Thank you again for
your time on this.

>
>> Tx queue switching may cause out-of-order packets.
>> Change sctp_packet_transmit to allow Tx queue switching only for
>> the first in flight packet, to avoid unwanted Tx queue switching.
>>
>> Signed-off-by: Wally Zhao <[email protected]>
>> ---
>> net/sctp/output.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index dbda7e7..5ff75cc 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet,
>> gfp_t gfp)
>> /* neighbour should be confirmed on successful transmission or
>> * positive error
>> */
>> +
>> + /* allow switch tx queue only for the first in flight pkt */
>> + head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;
>
> Considering we are talking about NIC queues here, we would have a
> better result with tp->flight_size instead. As in, we can switch
> queues if, for this transport, the queue is empty.
>
>> +
>> if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
>> tp->dst_pending_confirm)
>> tp->dst_pending_confirm = 0;
>> --
>> 1.8.3.1
>>
>

2019-10-30 19:12:07

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering

On Wed, Oct 30, 2019 at 11:54:45PM +0800, Wei Zhao wrote:
> On 10/30/19, Marcelo Ricardo Leitner <[email protected]> wrote:
> > On Wed, Oct 30, 2019 at 12:07:17PM -0400, Wally Zhao wrote:
> >> Unlike tcp_transmit_skb,
> >> sctp_packet_transmit does not set ooo_okay explicitly,
> >> causing unwanted Tx queue switching when multiqueue is in use;
> >
> > It is initialized to 0 by __alloc_skb() via:
> > memset(skb, 0, offsetof(struct sk_buff, tail));
> > and never set to 1 by anyone for SCTP.
> >
> > The patch description seems off. I don't see how the unwanted Tx queue
> > switching can happen. IOW, it's not fixing it OOO packets, but
> > improving it by allowing switching on the first packet. Am I missing
> > something?
>
> Thanks for pointing this out. You are right. This ooo_okay is default to false.
>
> I was observing some Tx queue switching before when testing with
> iperf3 (modified to be able to set window size, for higher throughput
> with long RTT), so I thought ooo_okay was set to true somewhere else
> after allocation. Just now I did the test again, it turns out that
> iperf3 made a re-connect silently which caused the Tx queue change.

Ah, okay.

>
> As for the improving purpose of this patch, that is not that critical
> from my side, and the patch description is not correct for this
> purpose. So I will give up this patch attempt. Thank you again for
> your time on this.

As you wish. If you don't have the time for it, ok, but the
improvement is welcomed. With a more accurate description and using
tp->flight_size instead, it should be good.

Thanks,
Marcelo

>
> >
> >> Tx queue switching may cause out-of-order packets.
> >> Change sctp_packet_transmit to allow Tx queue switching only for
> >> the first in flight packet, to avoid unwanted Tx queue switching.
> >>
> >> Signed-off-by: Wally Zhao <[email protected]>
> >> ---
> >> net/sctp/output.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/net/sctp/output.c b/net/sctp/output.c
> >> index dbda7e7..5ff75cc 100644
> >> --- a/net/sctp/output.c
> >> +++ b/net/sctp/output.c
> >> @@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet,
> >> gfp_t gfp)
> >> /* neighbour should be confirmed on successful transmission or
> >> * positive error
> >> */
> >> +
> >> + /* allow switch tx queue only for the first in flight pkt */
> >> + head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;
> >
> > Considering we are talking about NIC queues here, we would have a
> > better result with tp->flight_size instead. As in, we can switch
> > queues if, for this transport, the queue is empty.
> >
> >> +
> >> if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> >> tp->dst_pending_confirm)
> >> tp->dst_pending_confirm = 0;
> >> --
> >> 1.8.3.1
> >>
> >

2019-10-30 19:16:18

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering



On 10/30/19 9:07 AM, Wally Zhao wrote:
> Unlike tcp_transmit_skb,
> sctp_packet_transmit does not set ooo_okay explicitly,
> causing unwanted Tx queue switching when multiqueue is in use;
> Tx queue switching may cause out-of-order packets.
> Change sctp_packet_transmit to allow Tx queue switching only for
> the first in flight packet, to avoid unwanted Tx queue switching.
>

While the patch seems fine, the changelog is quite confusing.

When skb->ooo_olay is 0 (which is the default for freshly allocated skbs),
the core networking stack will stick to whatever TX queue was chosen
at the time the dst_entry was attached to the (connected) socket.

This means no reorder can happen at all by default.

By setting ooo_okay carefully (as you did in your patch), you allow
core networking stack to _switch_ to another TX queue based on
current CPU (XPS selection)

So even without your fix, SCTP should not experience out-of-order packets.

> Signed-off-by: Wally Zhao <[email protected]>
> ---
> net/sctp/output.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index dbda7e7..5ff75cc 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> +
> + /* allow switch tx queue only for the first in flight pkt */
> + head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;
> +
> if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
>

2019-10-31 01:12:54

by Wei Zhao

[permalink] [raw]
Subject: Re: [PATCH] sctp: set ooo_okay properly for Transmit Packet Steering

On Thu, Oct 31, 2019 at 3:03 AM Eric Dumazet <[email protected]> wrote:
>
>
>
> On 10/30/19 9:07 AM, Wally Zhao wrote:
> > Unlike tcp_transmit_skb,
> > sctp_packet_transmit does not set ooo_okay explicitly,
> > causing unwanted Tx queue switching when multiqueue is in use;
> > Tx queue switching may cause out-of-order packets.
> > Change sctp_packet_transmit to allow Tx queue switching only for
> > the first in flight packet, to avoid unwanted Tx queue switching.
> >
>
> While the patch seems fine, the changelog is quite confusing.
>
> When skb->ooo_olay is 0 (which is the default for freshly allocated skbs),
> the core networking stack will stick to whatever TX queue was chosen
> at the time the dst_entry was attached to the (connected) socket.
>
> This means no reorder can happen at all by default.
>
> By setting ooo_okay carefully (as you did in your patch), you allow
> core networking stack to _switch_ to another TX queue based on
> current CPU (XPS selection)
>
> So even without your fix, SCTP should not experience out-of-order packets.

Yes, you are right, as Marcelo also pointed out.
The changelog was given based on incorrect observation of a test
result, as I replied to Marcelo.
Since ooo_okay is default to 0, this is good enough; no need for any
patch from my side.
Thank you for your time on this.


>
> > Signed-off-by: Wally Zhao <[email protected]>
> > ---
> > net/sctp/output.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/sctp/output.c b/net/sctp/output.c
> > index dbda7e7..5ff75cc 100644
> > --- a/net/sctp/output.c
> > +++ b/net/sctp/output.c
> > @@ -626,6 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> > /* neighbour should be confirmed on successful transmission or
> > * positive error
> > */
> > +
> > + /* allow switch tx queue only for the first in flight pkt */
> > + head->ooo_okay = asoc->outqueue.outstanding_bytes == 0;
> > +
> > if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> > tp->dst_pending_confirm)
> > tp->dst_pending_confirm = 0;
> >

2019-11-04 08:48:17

by Chen, Rong A

[permalink] [raw]
Subject: [sctp] 327fecdaf3: BUG:kernel_NULL_pointer_dereference,address

FYI, we noticed the following commit (built with gcc-7):

commit: 327fecdaf39ab7163e6d189e19abdadf4555b002 ("[PATCH] sctp: set ooo_okay properly for Transmit Packet Steering")
url: https://github.com/0day-ci/linux/commits/Wally-Zhao/sctp-set-ooo_okay-properly-for-Transmit-Packet-Steering/20191101-171354


in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+-------------------------------------------------------+------------+------------+
| | 52340b82cf | 327fecdaf3 |
+-------------------------------------------------------+------------+------------+
| boot_successes | 32 | 48 |
| boot_failures | 3 | 28 |
| BUG:kernel_hang_in_test_stage | 3 | 14 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 14 |
| Oops:#[##] | 0 | 14 |
| RIP:sctp_packet_transmit | 0 | 14 |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 0 | 13 |
+-------------------------------------------------------+------------+------------+


If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 35.312661] BUG: kernel NULL pointer dereference, address: 00000000000005d8
[ 35.316225] #PF: supervisor read access in kernel mode
[ 35.319178] #PF: error_code(0x0000) - not-present page
[ 35.322078] PGD 800000021b569067 P4D 800000021b569067 PUD 21b688067 PMD 0
[ 35.325629] Oops: 0000 [#1] SMP PTI
[ 35.327965] CPU: 0 PID: 3148 Comm: trinity-c5 Not tainted 5.4.0-rc3-01107-g327fecdaf39ab #12
[ 35.332863] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 35.337932] RIP: 0010:sctp_packet_transmit+0x767/0x822
[ 35.340818] Code: 8a 45 40 d0 e8 83 e0 08 88 c2 8a 43 78 83 e0 f7 09 d0 88 43 78 41 f6 45 24 08 74 04 80 4b 7a 20 48 8b 04 24 4c 89 ee 48 89 df <83> b8 d8 05 00 00 00 0f 94 c0 c1 e0 07 88 c2 8a 43 78 83 e0 7f 09
[ 35.350270] RSP: 0000:ffffc90000003ac0 EFLAGS: 00010246
[ 35.353243] RAX: 0000000000000000 RBX: ffff888219803e00 RCX: 0000000000000007
[ 35.356838] RDX: 0000000000000000 RSI: ffff888218842800 RDI: ffff888219803e00
[ 35.360408] RBP: ffff888218842a10 R08: 00000000000002c0 R09: ffffffff81bb406d
[ 35.364054] R10: 0000000000000034 R11: 000000007f000001 R12: ffff888219802700
[ 35.367715] R13: ffff888218842800 R14: 0000000000000000 R15: ffff888219803e00
[ 35.371276] FS: 0000000000000000(0000) GS:ffff88823fc00000(0063) knlGS:000000000a47b880
[ 35.376104] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 35.379260] CR2: 00000000000005d8 CR3: 0000000218ee2000 CR4: 00000000000406f0
[ 35.382827] DR0: fffffffff7277000 DR1: 0000000000000000 DR2: 0000000000000000
[ 35.386346] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
[ 35.389854] Call Trace:
[ 35.391837] <IRQ>
[ 35.393688] ? sctp_sf_do_5_1B_init+0x153/0x2b1
[ 35.396438] sctp_do_sm+0xa52/0x149b
[ 35.398853] ? tcp_rcv_established+0x345/0x396
[ 35.401502] ? tcp_v4_do_rcv+0x9b/0x167
[ 35.403981] ? tcp_v4_rcv+0x54a/0x778
[ 35.406396] ? raw_local_deliver+0x1dc/0x22c
[ 35.409022] ? raw_local_deliver+0x1f1/0x22c
[ 35.411594] ? update_group_capacity+0x23/0x193
[ 35.414340] ? cpumask_next_and+0x19/0x1a
[ 35.416826] ? update_sd_lb_stats+0x4ae/0x4d2
[ 35.419471] sctp_endpoint_bh_rcv+0x19d/0x1e1
[ 35.422148] sctp_rcv+0x98b/0xa9b
[ 35.424311] ? sock_def_readable+0x4d/0x57
[ 35.426726] ? __sock_queue_rcv_skb+0x1ad/0x1bc
[ 35.429382] ? ip_protocol_deliver_rcu+0x76/0xeb
[ 35.432068] ? __sctp_lookup_association+0x24/0x24
[ 35.434839] ip_protocol_deliver_rcu+0x76/0xeb
[ 35.437474] ip_local_deliver+0x8a/0x96
[ 35.439960] __netif_receive_skb_one_core+0x71/0x93
[ 35.442693] process_backlog+0x86/0x125
[ 35.445115] net_rx_action+0x11e/0x2b9
[ 35.447605] __do_softirq+0x151/0x2dd
[ 35.450085] do_softirq_own_stack+0x2a/0x40
[ 35.452746] </IRQ>
[ 35.454643] do_softirq+0x3f/0x56
[ 35.456910] __local_bh_enable_ip+0x49/0x58
[ 35.459619] ip_finish_output2+0x30a/0x36c
[ 35.462217] ? ip_skb_dst_mtu+0x53/0x83
[ 35.464616] ? __ip_queue_xmit+0x2a8/0x2e0
[ 35.467261] __ip_queue_xmit+0x2a8/0x2e0
[ 35.469775] ? slab_free_freelist_hook+0x19/0x68
[ 35.472823] sctp_packet_transmit+0x790/0x822
[ 35.475401] ? sctp_packet_config+0x10c/0x1fa
[ 35.478101] sctp_outq_flush_ctrl+0x124/0x20f
[ 35.481001] sctp_outq_flush+0x66/0x647
[ 35.483502] sctp_do_sm+0x1309/0x149b
[ 35.485818] ? trace_hardirqs_on+0x2e/0x3a
[ 35.488309] ? ___slab_alloc+0xcb/0x3fe
[ 35.490698] ? sctp_stream_init_ext+0x2c/0x8a
[ 35.493147] ? sctp_ulpq_tail_event+0x191/0x1a8
[ 35.495543] ? tracer_hardirqs_on+0x1b/0xf6
[ 35.497999] sctp_primitive_ASSOCIATE+0x33/0x36
[ 35.500526] sctp_sendmsg_to_asoc+0x2f2/0x48a
[ 35.502810] ? sctp_connect_new_asoc+0xe1/0x176
[ 35.505001] sctp_sendmsg+0x717/0x795
[ 35.507337] ? memblock_search_pfn_nid+0x4b/0x5f
[ 35.509960] ? sock_sendmsg_nosec+0x2b/0x3c
[ 35.512459] sock_sendmsg_nosec+0x2b/0x3c
[ 35.514721] ___sys_sendmsg+0x1a8/0x22f
[ 35.516894] ? ___might_sleep+0x3b/0x144
[ 35.519058] ? ___might_sleep+0x3b/0x144
[ 35.520991] ? timerqueue_add+0x5e/0x62
[ 35.522512] ? enqueue_hrtimer+0x90/0x9a
[ 35.524303] ? _raw_spin_unlock_irqrestore+0x19/0x2b
[ 35.526195] ? hrtimer_start_range_ns+0x1eb/0x20f
[ 35.527856] ? tracer_hardirqs_on+0x1b/0xf6
[ 35.529383] ? __sys_sendmsg+0x5e/0x91
[ 35.531387] __sys_sendmsg+0x5e/0x91
[ 35.533398] do_int80_syscall_32+0x58/0x65
[ 35.535156] entry_INT80_compat+0x87/0xa0
[ 35.537606] Modules linked in:
[ 35.539765] CR2: 00000000000005d8
[ 35.541952] ---[ end trace 1e09b73b4a0b8b3e ]---


To reproduce:

# build kernel
cd linux
cp config-5.4.0-rc3-01107-g327fecdaf39ab .config
make HOSTCC=gcc-7 CC=gcc-7 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email



Thanks,
Rong Chen


Attachments:
(No filename) (6.45 kB)
config-5.4.0-rc3-01107-g327fecdaf39ab (121.02 kB)
job-script (4.64 kB)
dmesg.xz (17.23 kB)
Download all attachments

2019-11-04 13:27:07

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: [sctp] 327fecdaf3: BUG:kernel_NULL_pointer_dereference,address

On Mon, Nov 04, 2019 at 04:46:35PM +0800, kernel test robot wrote:
> [ 35.312661] BUG: kernel NULL pointer dereference, address: 00000000000005d8
> [ 35.316225] #PF: supervisor read access in kernel mode
> [ 35.319178] #PF: error_code(0x0000) - not-present page
> [ 35.322078] PGD 800000021b569067 P4D 800000021b569067 PUD 21b688067 PMD 0
> [ 35.325629] Oops: 0000 [#1] SMP PTI
> [ 35.327965] CPU: 0 PID: 3148 Comm: trinity-c5 Not tainted 5.4.0-rc3-01107-g327fecdaf39ab #12
> [ 35.332863] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 35.337932] RIP: 0010:sctp_packet_transmit+0x767/0x822

Right, as asoc can be NULL by then. (per the check on it a few lines
before the change here).

Marcelo

2019-11-04 14:14:56

by Wei Zhao

[permalink] [raw]
Subject: Re: [sctp] 327fecdaf3: BUG:kernel_NULL_pointer_dereference,address

On 11/4/19, Marcelo Ricardo Leitner <[email protected]> wrote:
> On Mon, Nov 04, 2019 at 04:46:35PM +0800, kernel test robot wrote:
>> [ 35.312661] BUG: kernel NULL pointer dereference, address:
>> 00000000000005d8
>> [ 35.316225] #PF: supervisor read access in kernel mode
>> [ 35.319178] #PF: error_code(0x0000) - not-present page
>> [ 35.322078] PGD 800000021b569067 P4D 800000021b569067 PUD 21b688067 PMD
>> 0
>> [ 35.325629] Oops: 0000 [#1] SMP PTI
>> [ 35.327965] CPU: 0 PID: 3148 Comm: trinity-c5 Not tainted
>> 5.4.0-rc3-01107-g327fecdaf39ab #12
>> [ 35.332863] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> 1.10.2-1 04/01/2014
>> [ 35.337932] RIP: 0010:sctp_packet_transmit+0x767/0x822
>
> Right, as asoc can be NULL by then. (per the check on it a few lines
> before the change here).

Yes, apologize for missing the NULL check (Actually I realized some
further check is need to correctly identify the first in flight
packet, as outstanding_bytes has already been increased by this first
in flight packet itself before getting into sctp_packet_transmit).

Anyway, I think I do not need further action, as the patch is anyway
not going to be merged, the 0day robot picks up the patch from the
mail list directly instead of git repo, right?

Thanks a lot,
Wally

>
> Marcelo
>

2019-11-04 14:52:35

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: [sctp] 327fecdaf3: BUG:kernel_NULL_pointer_dereference,address

On Mon, Nov 04, 2019 at 10:14:00PM +0800, Wei Zhao wrote:
> On 11/4/19, Marcelo Ricardo Leitner <[email protected]> wrote:
> > On Mon, Nov 04, 2019 at 04:46:35PM +0800, kernel test robot wrote:
> >> [ 35.312661] BUG: kernel NULL pointer dereference, address:
> >> 00000000000005d8
> >> [ 35.316225] #PF: supervisor read access in kernel mode
> >> [ 35.319178] #PF: error_code(0x0000) - not-present page
> >> [ 35.322078] PGD 800000021b569067 P4D 800000021b569067 PUD 21b688067 PMD
> >> 0
> >> [ 35.325629] Oops: 0000 [#1] SMP PTI
> >> [ 35.327965] CPU: 0 PID: 3148 Comm: trinity-c5 Not tainted
> >> 5.4.0-rc3-01107-g327fecdaf39ab #12
> >> [ 35.332863] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> >> 1.10.2-1 04/01/2014
> >> [ 35.337932] RIP: 0010:sctp_packet_transmit+0x767/0x822
> >
> > Right, as asoc can be NULL by then. (per the check on it a few lines
> > before the change here).
>
> Yes, apologize for missing the NULL check (Actually I realized some
> further check is need to correctly identify the first in flight
> packet, as outstanding_bytes has already been increased by this first
> in flight packet itself before getting into sctp_packet_transmit).
>
> Anyway, I think I do not need further action, as the patch is anyway
> not going to be merged, the 0day robot picks up the patch from the
> mail list directly instead of git repo, right?

That's my understanding as well. I double checked and the patch wasn't
applied by Dave, so we're good.

Thanks,
Marcelo