| Message ID | 20190226121235.20784-2-borisp@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | tls: Fix issues in tls_device | expand |
> -----Original Message----- > From: Boris Pismenny <borisp@mellanox.com> > Sent: Tuesday, February 26, 2019 5:43 PM > To: aviadye@mellanox.com; davejwatson@fb.com; > john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg > <vakul.garg@nxp.com>; netdev@vger.kernel.org > Cc: eranbe@mellanox.com; borisp@mellanox.com > Subject: [PATCH net 1/4] tls: Fix tls_device handling of partial records > > Cleanup the handling of partial records while fixing a bug where the > tls_push_pending_closed_record function is using the software tls > context instead of the hardware context. Can you provide details of what cleanup has been done? I see that we got rid of concept of 'TLS_PENDING_CLOSED_RECORD'. I vaguely remember that at one point in time, it seemed to me redundant. But I was not sure. Please confirm if it is the case. Can this patch be split into two? One for the cleanup and one for the bug. > > The bug resulted in the following crash: > [ 88.791229] BUG: unable to handle kernel NULL pointer dereference at > 0000000000000000 > [ 88.793271] #PF error: [normal kernel read fault] > [ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD > 22a156067 PMD 0 > [ 88.795958] Oops: 0000 [#1] SMP PTI > [ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3 > [ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS Bochs 01/01/2011 > [ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls] > [ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd > 4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 > c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 > [ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213 > [ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: > 0000000000000000 > [ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: > ffff9af1e88a1980 > [ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: > ffff9af1ebeeb700 > [ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: > 0000000000000000 > [ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: > 0000000000000000 > [ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) > knlGS:0000000000000000 > [ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: > 00000000001406e0 > [ 88.819328] Call Trace: > [ 88.820123] tls_push_data+0x628/0x6a0 [tls] > [ 88.821283] ? remove_wait_queue+0x20/0x60 > [ 88.822383] ? n_tty_read+0x683/0x910 > [ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls] > [ 88.824505] sock_sendmsg+0x36/0x50 > [ 88.825492] sock_write_iter+0x87/0x100 > [ 88.826521] __vfs_write+0x127/0x1b0 > [ 88.827499] vfs_write+0xad/0x1b0 > [ 88.828454] ksys_write+0x52/0xc0 > [ 88.829378] do_syscall_64+0x5b/0x180 > [ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 88.831603] RIP: 0033:0x7fcad1451680 > > [ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at > 0000000000000000 > [ 1248.472564] #PF error: [normal kernel read fault] > [ 1248.473790] PGD 0 P4D 0 > [ 1248.474642] Oops: 0000 [#1] SMP PTI > [ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0- > rc4+ #3 > [ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS Bochs 01/01/2011 > [ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls] > [ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c > 1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 > c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 > [ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213 > [ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: > 0000000000000006 > [ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: > 0000000000000286 > [ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: > 00000000000002b1 > [ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: > 0000000000000000 > [ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: > 0000000000000000 > [ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) > knlGS:0000000000000000 > [ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: > 00000000001406e0 > [ 1248.500136] Call Trace: > [ 1248.500998] ? tcp_check_oom+0xd0/0xd0 > [ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls] > [ 1248.503411] inet_release+0x3c/0x60 > [ 1248.504530] __sock_release+0x3d/0xb0 > [ 1248.505611] sock_close+0x11/0x20 > [ 1248.506612] __fput+0xb4/0x220 > [ 1248.507559] task_work_run+0x88/0xa0 > [ 1248.508617] do_exit+0x2cb/0xbc0 > [ 1248.509597] ? core_sys_select+0x17a/0x280 > [ 1248.510740] do_group_exit+0x39/0xb0 > [ 1248.511789] get_signal+0x1d0/0x630 > [ 1248.512823] do_signal+0x36/0x620 > [ 1248.513822] exit_to_usermode_loop+0x5c/0xc6 > [ 1248.515003] do_syscall_64+0x157/0x180 > [ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 1248.517456] RIP: 0033:0x7fb398bd3f53 > [ 1248.518537] Code: Bad RIP value. > > Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records > for performance") > Signed-off-by: Boris Pismenny <borisp@mellanox.com> > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> > --- > include/net/tls.h | 20 ++++---------------- > net/tls/tls_device.c | 9 +++++---- > net/tls/tls_main.c | 13 ------------- > 3 files changed, 9 insertions(+), 33 deletions(-) > > diff --git a/include/net/tls.h b/include/net/tls.h > index 9f4117ae2297..a528a082da73 100644 > --- a/include/net/tls.h > +++ b/include/net/tls.h > @@ -199,10 +199,6 @@ struct tls_offload_context_tx { > (ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \ > TLS_DRIVER_STATE_SIZE) > > -enum { > - TLS_PENDING_CLOSED_RECORD > -}; > - > struct cipher_context { > char *iv; > char *rec_seq; > @@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context > *ctx, > int tls_push_partial_record(struct sock *sk, struct tls_context *ctx, > int flags); > > -int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx, > - int flags, long *timeo); > - > static inline struct tls_msg *tls_msg(struct sk_buff *skb) > { > return (struct tls_msg *)strp_msg(skb); > } > > -static inline bool tls_is_pending_closed_record(struct tls_context *ctx) > +static inline bool tls_is_partially_sent_record(struct tls_context *ctx) > { > - return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); > + return !!ctx->partially_sent_record; > } > > static inline int tls_complete_pending_work(struct sock *sk, > @@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct > sock *sk, > if (unlikely(sk->sk_write_pending)) > rc = wait_on_pending_writer(sk, timeo); > > - if (!rc && tls_is_pending_closed_record(ctx)) > - rc = tls_push_pending_closed_record(sk, ctx, flags, timeo); > + if (!rc && tls_is_partially_sent_record(ctx)) > + rc = tls_push_partial_record(sk, ctx, flags); > > return rc; > } > > -static inline bool tls_is_partially_sent_record(struct tls_context *ctx) > -{ > - return !!ctx->partially_sent_record; > -} > - > static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx) > { > return tls_ctx->pending_open_record_frags; > diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c > index a5c17c47d08a..3e5e8e021a87 100644 > --- a/net/tls/tls_device.c > +++ b/net/tls/tls_device.c > @@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk, > list_add_tail(&record->list, &offload_ctx->records_list); > spin_unlock_irq(&offload_ctx->lock); > offload_ctx->open_record = NULL; > - set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); > tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version); > > for (i = 0; i < record->num_frags; i++) { > @@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk, > return -sk->sk_err; > > timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT); > - rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo); > - if (rc < 0) > - return rc; > + if (tls_is_partially_sent_record(tls_ctx)) { > + rc = tls_push_partial_record(sk, tls_ctx, flags); > + if (rc < 0) > + return rc; > + } > > pfrag = sk_page_frag(sk); > > diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c > index caff15b2f9b2..7e05af75536d 100644 > --- a/net/tls/tls_main.c > +++ b/net/tls/tls_main.c > @@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct > tls_context *ctx, > return tls_push_sg(sk, ctx, sg, offset, flags); > } > > -int tls_push_pending_closed_record(struct sock *sk, > - struct tls_context *tls_ctx, > - int flags, long *timeo) > -{ > - struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); > - > - if (tls_is_partially_sent_record(tls_ctx) || > - !list_empty(&ctx->tx_list)) > - return tls_tx_records(sk, flags); > - else > - return tls_ctx->push_pending_record(sk, flags); > -} > - > static void tls_write_space(struct sock *sk) > { > struct tls_context *ctx = tls_get_ctx(sk); > -- > 2.12.2
On 2/26/2019 4:57 PM, Vakul Garg wrote: > > >> -----Original Message----- >> From: Boris Pismenny <borisp@mellanox.com> >> Sent: Tuesday, February 26, 2019 5:43 PM >> To: aviadye@mellanox.com; davejwatson@fb.com; >> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg >> <vakul.garg@nxp.com>; netdev@vger.kernel.org >> Cc: eranbe@mellanox.com; borisp@mellanox.com >> Subject: [PATCH net 1/4] tls: Fix tls_device handling of partial records >> >> Cleanup the handling of partial records while fixing a bug where the >> tls_push_pending_closed_record function is using the software tls >> context instead of the hardware context. > > Can you provide details of what cleanup has been done? > I see that we got rid of concept of 'TLS_PENDING_CLOSED_RECORD'. > I vaguely remember that at one point in time, it seemed to me redundant. > But I was not sure. Please confirm if it is the case. > The cleanup refers to the PENDING_CLOSED_RECORD. This code was previously used by both tls_sw and tls_device to handle the closed records. However, at some point tls_sw moved to using the partially sent record code, which is equivalent. So this code became unused after we fixed the tls_device code path, and this is why it is removed here. > Can this patch be split into two? One for the cleanup and one for the bug. > The bug fix will cause the PENDING_CLOSED_RECORD code to be unused. IMO, it is better to keep this as-is to avoid this. >> >> The bug resulted in the following crash: >> [ 88.791229] BUG: unable to handle kernel NULL pointer dereference at >> 0000000000000000 >> [ 88.793271] #PF error: [normal kernel read fault] >> [ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD >> 22a156067 PMD 0 >> [ 88.795958] Oops: 0000 [#1] SMP PTI >> [ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3 >> [ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), >> BIOS Bochs 01/01/2011 >> [ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls] >> [ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd >> 4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 >> c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 >> [ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213 >> [ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: >> 0000000000000000 >> [ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: >> ffff9af1e88a1980 >> [ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: >> ffff9af1ebeeb700 >> [ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: >> 0000000000000000 >> [ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: >> 0000000000000000 >> [ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) >> knlGS:0000000000000000 >> [ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> [ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: >> 00000000001406e0 >> [ 88.819328] Call Trace: >> [ 88.820123] tls_push_data+0x628/0x6a0 [tls] >> [ 88.821283] ? remove_wait_queue+0x20/0x60 >> [ 88.822383] ? n_tty_read+0x683/0x910 >> [ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls] >> [ 88.824505] sock_sendmsg+0x36/0x50 >> [ 88.825492] sock_write_iter+0x87/0x100 >> [ 88.826521] __vfs_write+0x127/0x1b0 >> [ 88.827499] vfs_write+0xad/0x1b0 >> [ 88.828454] ksys_write+0x52/0xc0 >> [ 88.829378] do_syscall_64+0x5b/0x180 >> [ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> [ 88.831603] RIP: 0033:0x7fcad1451680 >> >> [ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at >> 0000000000000000 >> [ 1248.472564] #PF error: [normal kernel read fault] >> [ 1248.473790] PGD 0 P4D 0 >> [ 1248.474642] Oops: 0000 [#1] SMP PTI >> [ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0- >> rc4+ #3 >> [ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), >> BIOS Bochs 01/01/2011 >> [ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls] >> [ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c >> 1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 >> c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 >> [ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213 >> [ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: >> 0000000000000006 >> [ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: >> 0000000000000286 >> [ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: >> 00000000000002b1 >> [ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: >> 0000000000000000 >> [ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: >> 0000000000000000 >> [ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) >> knlGS:0000000000000000 >> [ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> [ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: >> 00000000001406e0 >> [ 1248.500136] Call Trace: >> [ 1248.500998] ? tcp_check_oom+0xd0/0xd0 >> [ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls] >> [ 1248.503411] inet_release+0x3c/0x60 >> [ 1248.504530] __sock_release+0x3d/0xb0 >> [ 1248.505611] sock_close+0x11/0x20 >> [ 1248.506612] __fput+0xb4/0x220 >> [ 1248.507559] task_work_run+0x88/0xa0 >> [ 1248.508617] do_exit+0x2cb/0xbc0 >> [ 1248.509597] ? core_sys_select+0x17a/0x280 >> [ 1248.510740] do_group_exit+0x39/0xb0 >> [ 1248.511789] get_signal+0x1d0/0x630 >> [ 1248.512823] do_signal+0x36/0x620 >> [ 1248.513822] exit_to_usermode_loop+0x5c/0xc6 >> [ 1248.515003] do_syscall_64+0x157/0x180 >> [ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> [ 1248.517456] RIP: 0033:0x7fb398bd3f53 >> [ 1248.518537] Code: Bad RIP value. >> >> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records >> for performance") >> Signed-off-by: Boris Pismenny <borisp@mellanox.com> >> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> >> --- >> include/net/tls.h | 20 ++++---------------- >> net/tls/tls_device.c | 9 +++++---- >> net/tls/tls_main.c | 13 ------------- >> 3 files changed, 9 insertions(+), 33 deletions(-) >> >> diff --git a/include/net/tls.h b/include/net/tls.h >> index 9f4117ae2297..a528a082da73 100644 >> --- a/include/net/tls.h >> +++ b/include/net/tls.h >> @@ -199,10 +199,6 @@ struct tls_offload_context_tx { >> (ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \ >> TLS_DRIVER_STATE_SIZE) >> >> -enum { >> - TLS_PENDING_CLOSED_RECORD >> -}; >> - >> struct cipher_context { >> char *iv; >> char *rec_seq; >> @@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context >> *ctx, >> int tls_push_partial_record(struct sock *sk, struct tls_context *ctx, >> int flags); >> >> -int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx, >> - int flags, long *timeo); >> - >> static inline struct tls_msg *tls_msg(struct sk_buff *skb) >> { >> return (struct tls_msg *)strp_msg(skb); >> } >> >> -static inline bool tls_is_pending_closed_record(struct tls_context *ctx) >> +static inline bool tls_is_partially_sent_record(struct tls_context *ctx) >> { >> - return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); >> + return !!ctx->partially_sent_record; >> } >> >> static inline int tls_complete_pending_work(struct sock *sk, >> @@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct >> sock *sk, >> if (unlikely(sk->sk_write_pending)) >> rc = wait_on_pending_writer(sk, timeo); >> >> - if (!rc && tls_is_pending_closed_record(ctx)) >> - rc = tls_push_pending_closed_record(sk, ctx, flags, timeo); >> + if (!rc && tls_is_partially_sent_record(ctx)) >> + rc = tls_push_partial_record(sk, ctx, flags); >> >> return rc; >> } >> >> -static inline bool tls_is_partially_sent_record(struct tls_context *ctx) >> -{ >> - return !!ctx->partially_sent_record; >> -} >> - >> static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx) >> { >> return tls_ctx->pending_open_record_frags; >> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c >> index a5c17c47d08a..3e5e8e021a87 100644 >> --- a/net/tls/tls_device.c >> +++ b/net/tls/tls_device.c >> @@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk, >> list_add_tail(&record->list, &offload_ctx->records_list); >> spin_unlock_irq(&offload_ctx->lock); >> offload_ctx->open_record = NULL; >> - set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); >> tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version); >> >> for (i = 0; i < record->num_frags; i++) { >> @@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk, >> return -sk->sk_err; >> >> timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT); >> - rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo); >> - if (rc < 0) >> - return rc; >> + if (tls_is_partially_sent_record(tls_ctx)) { >> + rc = tls_push_partial_record(sk, tls_ctx, flags); >> + if (rc < 0) >> + return rc; >> + } >> >> pfrag = sk_page_frag(sk); >> >> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c >> index caff15b2f9b2..7e05af75536d 100644 >> --- a/net/tls/tls_main.c >> +++ b/net/tls/tls_main.c >> @@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct >> tls_context *ctx, >> return tls_push_sg(sk, ctx, sg, offset, flags); >> } >> >> -int tls_push_pending_closed_record(struct sock *sk, >> - struct tls_context *tls_ctx, >> - int flags, long *timeo) >> -{ >> - struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); >> - >> - if (tls_is_partially_sent_record(tls_ctx) || >> - !list_empty(&ctx->tx_list)) >> - return tls_tx_records(sk, flags); >> - else >> - return tls_ctx->push_pending_record(sk, flags); >> -} >> - >> static void tls_write_space(struct sock *sk) >> { >> struct tls_context *ctx = tls_get_ctx(sk); >> -- >> 2.12.2 >
diff --git a/include/net/tls.h b/include/net/tls.h index 9f4117ae2297..a528a082da73 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -199,10 +199,6 @@ struct tls_offload_context_tx { (ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \ TLS_DRIVER_STATE_SIZE) -enum { - TLS_PENDING_CLOSED_RECORD -}; - struct cipher_context { char *iv; char *rec_seq; @@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context *ctx, int tls_push_partial_record(struct sock *sk, struct tls_context *ctx, int flags); -int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx, - int flags, long *timeo); - static inline struct tls_msg *tls_msg(struct sk_buff *skb) { return (struct tls_msg *)strp_msg(skb); } -static inline bool tls_is_pending_closed_record(struct tls_context *ctx) +static inline bool tls_is_partially_sent_record(struct tls_context *ctx) { - return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); + return !!ctx->partially_sent_record; } static inline int tls_complete_pending_work(struct sock *sk, @@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct sock *sk, if (unlikely(sk->sk_write_pending)) rc = wait_on_pending_writer(sk, timeo); - if (!rc && tls_is_pending_closed_record(ctx)) - rc = tls_push_pending_closed_record(sk, ctx, flags, timeo); + if (!rc && tls_is_partially_sent_record(ctx)) + rc = tls_push_partial_record(sk, ctx, flags); return rc; } -static inline bool tls_is_partially_sent_record(struct tls_context *ctx) -{ - return !!ctx->partially_sent_record; -} - static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx) { return tls_ctx->pending_open_record_frags; diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index a5c17c47d08a..3e5e8e021a87 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk, list_add_tail(&record->list, &offload_ctx->records_list); spin_unlock_irq(&offload_ctx->lock); offload_ctx->open_record = NULL; - set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version); for (i = 0; i < record->num_frags; i++) { @@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk, return -sk->sk_err; timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT); - rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo); - if (rc < 0) - return rc; + if (tls_is_partially_sent_record(tls_ctx)) { + rc = tls_push_partial_record(sk, tls_ctx, flags); + if (rc < 0) + return rc; + } pfrag = sk_page_frag(sk); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index caff15b2f9b2..7e05af75536d 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx, return tls_push_sg(sk, ctx, sg, offset, flags); } -int tls_push_pending_closed_record(struct sock *sk, - struct tls_context *tls_ctx, - int flags, long *timeo) -{ - struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); - - if (tls_is_partially_sent_record(tls_ctx) || - !list_empty(&ctx->tx_list)) - return tls_tx_records(sk, flags); - else - return tls_ctx->push_pending_record(sk, flags); -} - static void tls_write_space(struct sock *sk) { struct tls_context *ctx = tls_get_ctx(sk);
Cleanup the handling of partial records while fixing a bug where the tls_push_pending_closed_record function is using the software tls context instead of the hardware context. The bug resulted in the following crash: [ 88.791229] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 [ 88.793271] #PF error: [normal kernel read fault] [ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD 22a156067 PMD 0 [ 88.795958] Oops: 0000 [#1] SMP PTI [ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3 [ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls] [ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd 4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 [ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213 [ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: 0000000000000000 [ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9af1e88a1980 [ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: ffff9af1ebeeb700 [ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 [ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: 0000000000000000 [ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) knlGS:0000000000000000 [ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: 00000000001406e0 [ 88.819328] Call Trace: [ 88.820123] tls_push_data+0x628/0x6a0 [tls] [ 88.821283] ? remove_wait_queue+0x20/0x60 [ 88.822383] ? n_tty_read+0x683/0x910 [ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls] [ 88.824505] sock_sendmsg+0x36/0x50 [ 88.825492] sock_write_iter+0x87/0x100 [ 88.826521] __vfs_write+0x127/0x1b0 [ 88.827499] vfs_write+0xad/0x1b0 [ 88.828454] ksys_write+0x52/0xc0 [ 88.829378] do_syscall_64+0x5b/0x180 [ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 88.831603] RIP: 0033:0x7fcad1451680 [ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 [ 1248.472564] #PF error: [normal kernel read fault] [ 1248.473790] PGD 0 P4D 0 [ 1248.474642] Oops: 0000 [#1] SMP PTI [ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0-rc4+ #3 [ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls] [ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c 1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39 c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00 [ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213 [ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: 0000000000000006 [ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: 0000000000000286 [ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: 00000000000002b1 [ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: 0000000000000000 [ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: 0000000000000000 [ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) knlGS:0000000000000000 [ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: 00000000001406e0 [ 1248.500136] Call Trace: [ 1248.500998] ? tcp_check_oom+0xd0/0xd0 [ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls] [ 1248.503411] inet_release+0x3c/0x60 [ 1248.504530] __sock_release+0x3d/0xb0 [ 1248.505611] sock_close+0x11/0x20 [ 1248.506612] __fput+0xb4/0x220 [ 1248.507559] task_work_run+0x88/0xa0 [ 1248.508617] do_exit+0x2cb/0xbc0 [ 1248.509597] ? core_sys_select+0x17a/0x280 [ 1248.510740] do_group_exit+0x39/0xb0 [ 1248.511789] get_signal+0x1d0/0x630 [ 1248.512823] do_signal+0x36/0x620 [ 1248.513822] exit_to_usermode_loop+0x5c/0xc6 [ 1248.515003] do_syscall_64+0x157/0x180 [ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 1248.517456] RIP: 0033:0x7fb398bd3f53 [ 1248.518537] Code: Bad RIP value. Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance") Signed-off-by: Boris Pismenny <borisp@mellanox.com> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> --- include/net/tls.h | 20 ++++---------------- net/tls/tls_device.c | 9 +++++---- net/tls/tls_main.c | 13 ------------- 3 files changed, 9 insertions(+), 33 deletions(-)