From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Fastabend Subject: Re: [PATCH net-next] Prevent overflow of sk_msg in sk_msg_clone() Date: Fri, 21 Dec 2018 08:19:36 -0800 Message-ID: References: <20181221155357.6337-1-vakul.garg@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "daniel@iogearbox.net" , "davejwatson@fb.com" , "davem@davemloft.net" To: Vakul Garg , "netdev@vger.kernel.org" Return-path: Received: from mail-it1-f195.google.com ([209.85.166.195]:39822 "EHLO mail-it1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731601AbeLUQTw (ORCPT ); Fri, 21 Dec 2018 11:19:52 -0500 Received: by mail-it1-f195.google.com with SMTP id a6so7279227itl.4 for ; Fri, 21 Dec 2018 08:19:52 -0800 (PST) In-Reply-To: <20181221155357.6337-1-vakul.garg@nxp.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 12/21/18 7:55 AM, Vakul Garg wrote: > Fixed function sk_msg_clone() to prevent overflow of 'dst' while adding > pages in scatterlist entries. The overflow of 'dst' causes crash in kernel > tls module while doing record encryption. > > Crash fixed by this patch. > > [ 78.796119] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 > [ 78.804900] Mem abort info: > [ 78.807683] ESR = 0x96000004 > [ 78.810744] Exception class = DABT (current EL), IL = 32 bits > [ 78.816677] SET = 0, FnV = 0 > [ 78.819727] EA = 0, S1PTW = 0 > [ 78.822873] Data abort info: > [ 78.825759] ISV = 0, ISS = 0x00000004 > [ 78.829600] CM = 0, WnR = 0 > [ 78.832576] user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000bf8ee311 > [ 78.839195] [0000000000000008] pgd=0000000000000000 > [ 78.844081] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 78.849642] Modules linked in: tls xt_conntrack ipt_REJECT nf_reject_ipv4 ip6table_filter ip6_tables xt_CHECKSUM cpve cpufreq_conservative lm90 ina2xx crct10dif_ce > [ 78.865377] CPU: 0 PID: 6007 Comm: openssl Not tainted 4.20.0-rc6-01647-g754d5da63145-dirty #107 > [ 78.874149] Hardware name: LS1043A RDB Board (DT) > [ 78.878844] pstate: 60000005 (nZCv daif -PAN -UAO) > [ 78.883632] pc : scatterwalk_copychunks+0x164/0x1c8 > [ 78.888500] lr : scatterwalk_copychunks+0x160/0x1c8 > [ 78.893366] sp : ffff00001d04b600 > [ 78.896668] x29: ffff00001d04b600 x28: ffff80006814c680 > [ 78.901970] x27: 0000000000000000 x26: ffff80006c8de786 > [ 78.907272] x25: ffff00001d04b760 x24: 000000000000001a > [ 78.912573] x23: 0000000000000006 x22: ffff80006814e440 > [ 78.917874] x21: 0000000000000100 x20: 0000000000000000 > [ 78.923175] x19: 000081ffffffffff x18: 0000000000000400 > [ 78.928476] x17: 0000000000000008 x16: 0000000000000000 > [ 78.933778] x15: 0000000000000100 x14: 0000000000000001 > [ 78.939079] x13: 0000000000001080 x12: 0000000000000020 > [ 78.944381] x11: 0000000000001080 x10: 00000000ffff0002 > [ 78.949683] x9 : ffff80006814c248 x8 : 00000000ffff0000 > [ 78.954985] x7 : ffff80006814c318 x6 : ffff80006c8de786 > [ 78.960286] x5 : 0000000000000f80 x4 : ffff80006c8de000 > [ 78.965588] x3 : 0000000000000000 x2 : 0000000000001086 > [ 78.970889] x1 : ffff7e0001b74e02 x0 : 0000000000000000 > [ 78.976192] Process openssl (pid: 6007, stack limit = 0x00000000291367f9) > [ 78.982968] Call trace: > [ 78.985406] scatterwalk_copychunks+0x164/0x1c8 > [ 78.989927] skcipher_walk_next+0x28c/0x448 > [ 78.994099] skcipher_walk_done+0xfc/0x258 > [ 78.998187] gcm_encrypt+0x434/0x4c0 > [ 79.001758] tls_push_record+0x354/0xa58 [tls] > [ 79.006194] bpf_exec_tx_verdict+0x1e4/0x3e8 [tls] > [ 79.010978] tls_sw_sendmsg+0x650/0x780 [tls] > [ 79.015326] inet_sendmsg+0x2c/0xf8 > [ 79.018806] sock_sendmsg+0x18/0x30 > [ 79.022284] __sys_sendto+0x104/0x138 > [ 79.025935] __arm64_sys_sendto+0x24/0x30 > [ 79.029936] el0_svc_common+0x60/0xe8 > [ 79.033588] el0_svc_handler+0x2c/0x80 > [ 79.037327] el0_svc+0x8/0xc > [ 79.040200] Code: 6b01005f 54fff788 940169b1 f9000320 (b9400801) > [ 79.046283] ---[ end trace 74db007d069c1cf7 ]--- > > Signed-off-by: Vakul Garg > Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface") Seems we only checked it upfront not as pages were being added. Thanks! Acked-by: John Fastabend