From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AF26BC43381 for ; Tue, 26 Feb 2019 14:57:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6A972217F5 for ; Tue, 26 Feb 2019 14:57:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=nxp.com header.i=@nxp.com header.b="Jd9G+gUe" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728014AbfBZO5n (ORCPT ); Tue, 26 Feb 2019 09:57:43 -0500 Received: from mail-eopbgr70072.outbound.protection.outlook.com ([40.107.7.72]:54668 "EHLO EUR04-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727506AbfBZO5n (ORCPT ); Tue, 26 Feb 2019 09:57:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+hkaN6t7mAIPXFKVuXv3rmxiezzgHXNKkMjMQSJUo6w=; b=Jd9G+gUeeCdzQHUxaOiCsHczZ/C2Y4IHRcVsSs5Wx3UqrLjgtKv4rTtdSE+5739Kq27WcJszzqeDtxoXNg3zg6KNo7VN4H04CPjDbNyLZ/aiEzBqXuUK5+Woh2wwxfWi8fbroQe+GjDnx5Hd5WACyIqkX4mFi5fa+4eErKE07Vo= Received: from DB7PR04MB4252.eurprd04.prod.outlook.com (52.135.131.26) by DB7PR04MB4154.eurprd04.prod.outlook.com (52.135.130.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.18; Tue, 26 Feb 2019 14:57:38 +0000 Received: from DB7PR04MB4252.eurprd04.prod.outlook.com ([fe80::9ca:d78:c154:25b5]) by DB7PR04MB4252.eurprd04.prod.outlook.com ([fe80::9ca:d78:c154:25b5%4]) with mapi id 15.20.1643.019; Tue, 26 Feb 2019 14:57:38 +0000 From: Vakul Garg To: Boris Pismenny , "aviadye@mellanox.com" , "davejwatson@fb.com" , "john.fastabend@gmail.com" , "daniel@iogearbox.net" , "netdev@vger.kernel.org" CC: "eranbe@mellanox.com" Subject: RE: [PATCH net 1/4] tls: Fix tls_device handling of partial records Thread-Topic: [PATCH net 1/4] tls: Fix tls_device handling of partial records Thread-Index: AQHUzcydsw60a70GrEmBwbXhc7JWVKXyKrjQ Date: Tue, 26 Feb 2019 14:57:38 +0000 Message-ID: References: <20190226121235.20784-1-borisp@mellanox.com> <20190226121235.20784-2-borisp@mellanox.com> In-Reply-To: <20190226121235.20784-2-borisp@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=vakul.garg@nxp.com; x-originating-ip: [160.202.37.40] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 24d8fd8c-7acb-4f3e-e452-08d69bfabfef x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);SRVR:DB7PR04MB4154; x-ms-traffictypediagnostic: DB7PR04MB4154: x-microsoft-exchange-diagnostics: =?us-ascii?Q?1;DB7PR04MB4154;23:eqRYXtAfhpHI6WgiqPyzQ3LPwOIvmchS0gHDj9erl?= =?us-ascii?Q?NruBLUcAQMteowVAb2FuwAXYPScuZvZCcJj3uleGnmTdMLFszSvtHZ/TrTe2?= =?us-ascii?Q?B5Xd+d7mRpT1Gy4mqC5Aaqa4iD7dsgh0+YSDEymma2Lk0MwgNMg9AFuCd7NV?= =?us-ascii?Q?e7V/zLZhLqs2/pxgjWZoY1UNloI/vtz2BZKE5gEvKSrKp6vO7dj5T9e8fYcR?= =?us-ascii?Q?7j/2+nWVsXHluRFvUogcwE7Mr2g9dUrdWYwTHPY6/L49MbBftVH5Hi6jDtTp?= =?us-ascii?Q?RYD4IZGY5DyqlBahxp342WUoxJ8Czbfcl9DltSt9hqjZRuNjyvOOlRB522Yq?= =?us-ascii?Q?rdeyLo+pR9KD3WoxHV2OnvRvacd6Baj0eeIINgxXH3X+lNdTlGwulEoRTMEs?= =?us-ascii?Q?9nZwa//T2mf6Hb9nnl+G9DAYi/7+YihDmUsbjLHlChhp7HhTSvFUhaRfizy9?= =?us-ascii?Q?ANQBEpu1h00UdsHP/XwltvbPk6806soAFUONlvn4kL4a094Qa/IF8NHh7/Lb?= =?us-ascii?Q?72QPDtvsCbdgafNJ+YwxOXlmEcoNGI2nzh/0bwgPaHBk9/iUpXT6nknTWHXo?= =?us-ascii?Q?UUTFBJTxfqD9nZBvUWbjiDxu6UO2//GD4m12dKHG58U8PcStl2bJotk2RV8b?= =?us-ascii?Q?l/NuIu6tcLy9a59LfHJcdQog7JXmE3nC1obiCMYGYzrNMeiUFqkB945RLEQG?= =?us-ascii?Q?trkaKedGtGXDp9bSu5D6clQDzPQEvYoyw8zHJNqIdOjAwJeKC0DEdZFG1HR+?= =?us-ascii?Q?GUoVhJlybqcZQ/Tzqh7lryWU+OAnncVhwsw5UF2Xr1ehzg+yZWCByZWLUe8s?= =?us-ascii?Q?Gt5DQemZUuU9M2fwUME9dpY3JTrvvbDi2YcsJgXaBORLGhFRFJF2KPozaMdi?= =?us-ascii?Q?ex9MaZjspYG9oJEKnmoHOpw9CTo6LyYXJeEMarHWQr6tam099A9nRrqN3DZo?= =?us-ascii?Q?TMlJA9rsQ6afv52byf8ERZxFNDR7H7LrEd2Ou8h6tG+DwP2bT8rl25nRb055?= =?us-ascii?Q?MyaWc8MpyUlNh+fvhSWAIL5QzfH6KRDiHqm3iepnCTU669BGYgXRrEuKyIdZ?= =?us-ascii?Q?B+ZRPq2fJbrI6jU1xMNS+OhPmfNC5C53Shvi8dKVjRE1wtLRVKw58k3PCNPO?= =?us-ascii?Q?73O53OaJFzgMzYgP6zbyONcbTFtUJxjsjpqVY8HzgXY4wHMsEAMpjTCva/fn?= =?us-ascii?Q?L1NEUiw9DURObTrONMYayyx3l6pS//TdY0gYca8k0eJwVCHpAj0FCkoquFtT?= =?us-ascii?Q?sz87KvmaFjd5A4E5DuNl/C1EMsgAGwXEj36wHwTUZ8TimOZhYmc3RQBWv3H/?= =?us-ascii?Q?lMSvBOE4nEQ5UEAMfXICj+N91jhSACQ+LLdhDe27kB9?= x-microsoft-antispam-prvs: x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(39860400002)(136003)(366004)(346002)(376002)(396003)(199004)(189003)(13464003)(6246003)(14454004)(97736004)(11346002)(446003)(476003)(2501003)(486006)(2201001)(4326008)(86362001)(81156014)(8936002)(33656002)(3846002)(25786009)(8676002)(6116002)(81166006)(44832011)(68736007)(99286004)(110136005)(105586002)(106356001)(66066001)(6506007)(316002)(6436002)(9686003)(55016002)(74316002)(478600001)(2906002)(305945005)(7736002)(229853002)(7696005)(26005)(186003)(102836004)(53546011)(53936002)(71190400001)(71200400001)(52536013)(14444005)(256004)(5660300002)(76176011)(309714004);DIR:OUT;SFP:1101;SCL:1;SRVR:DB7PR04MB4154;H:DB7PR04MB4252.eurprd04.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: whWE7yQWKTDZvbY11E2ANgux8efy1QGWY1l4GQmlVIUc5HM0HOBntwiZld2I4iolAt1/yE7LOqScVuPONac5Xw2CbpIYRTztTSF3zNxS+hRRf/1o8gsHi7ElngpqPWou0JLU3LyaTpAljqtUkjSvMaYIrk5+4PMjqEc/NtcsSdss7nwIAEF493oNKzuOWxcAQrTGIIJyoQd9HfjSywwpNo0bDnOzKwdEYDwGfER5e7UvSezTuAp/9Jd4JwyXQppL5sO1wKnHjjGji2lB09CviaZxqrDwuqTGByWxmyNwcl0dHkYA4sOniHH2KDkYpTkFc/2D02SBkSOuoczEtA8qgbgPreTu6+4gpgWx0ceZIKjtSwrV0lIV8OkWn4dTsWsIXR6bUHTGkW8baLI6btqUn64NZ+tNDS82+C8nRn49Kqo= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 24d8fd8c-7acb-4f3e-e452-08d69bfabfef X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2019 14:57:38.2722 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4154 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org > -----Original Message----- > From: Boris Pismenny > Sent: Tuesday, February 26, 2019 5:43 PM > To: aviadye@mellanox.com; davejwatson@fb.com; > john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg > ; netdev@vger.kernel.org > Cc: eranbe@mellanox.com; borisp@mellanox.com > Subject: [PATCH net 1/4] tls: Fix tls_device handling of partial records >=20 > 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. >=20 > 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 >=20 > [ 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. >=20 > Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of record= s > for performance") > Signed-off-by: Boris Pismenny > Signed-off-by: Eran Ben Elisha > --- > include/net/tls.h | 20 ++++---------------- > net/tls/tls_device.c | 9 +++++---- > net/tls/tls_main.c | 13 ------------- > 3 files changed, 9 insertions(+), 33 deletions(-) >=20 > 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) >=20 > -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); >=20 > -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); > } >=20 > -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; > } >=20 > 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 =3D wait_on_pending_writer(sk, timeo); >=20 > - if (!rc && tls_is_pending_closed_record(ctx)) > - rc =3D tls_push_pending_closed_record(sk, ctx, flags, timeo); > + if (!rc && tls_is_partially_sent_record(ctx)) > + rc =3D tls_push_partial_record(sk, ctx, flags); >=20 > return rc; > } >=20 > -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_ct= x) > { > 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 =3D NULL; > - set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags); > tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version); >=20 > for (i =3D 0; i < record->num_frags; i++) { > @@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk, > return -sk->sk_err; >=20 > timeo =3D sock_sndtimeo(sk, flags & MSG_DONTWAIT); > - rc =3D tls_complete_pending_work(sk, tls_ctx, flags, &timeo); > - if (rc < 0) > - return rc; > + if (tls_is_partially_sent_record(tls_ctx)) { > + rc =3D tls_push_partial_record(sk, tls_ctx, flags); > + if (rc < 0) > + return rc; > + } >=20 > pfrag =3D sk_page_frag(sk); >=20 > 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); > } >=20 > -int tls_push_pending_closed_record(struct sock *sk, > - struct tls_context *tls_ctx, > - int flags, long *timeo) > -{ > - struct tls_sw_context_tx *ctx =3D 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 =3D tls_get_ctx(sk); > -- > 2.12.2