From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752092AbcF1Hw2 (ORCPT ); Tue, 28 Jun 2016 03:52:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46618 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750871AbcF1HwZ (ORCPT ); Tue, 28 Jun 2016 03:52:25 -0400 Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: <1eeb00c5098d8096cdb61dc7ee1ddc61b3e80f9e.1466974736.git.luto@kernel.org> References: <1eeb00c5098d8096cdb61dc7ee1ddc61b3e80f9e.1466974736.git.luto@kernel.org> To: Andy Lutomirski Cc: dhowells@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Borislav Petkov , Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens , Herbert Xu Subject: Re: [PATCH v4 02/29] rxrpc: Avoid using stack memory in SG lists in rxkad MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <697.1467100340.1@warthog.procyon.org.uk> Date: Tue, 28 Jun 2016 08:52:20 +0100 Message-ID: <699.1467100340@warthog.procyon.org.uk> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 28 Jun 2016 07:52:24 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Andy Lutomirski wrote: > - skcipher_request_set_crypt(req, &sg[1], &sg[0], sizeof(tmpbuf), iv.x); > + skcipher_request_set_crypt(req, &sg, &sg, sizeof(tmpbuf), iv.x); Don't the sg's have to be different? Aren't they both altered by the process of reading/writing from them? > struct rxrpc_skb_priv *sp; > ... > + swap(tmpbuf.xl, *(__be64 *)sp); > + > + sg_init_one(&sg, sp, sizeof(tmpbuf)); ???? I assume you're assuming that the rxrpc_skb_priv struct contents can arbitrarily replaced temporarily... And using an XCHG-equivalent instruction? This won't work on a 32-bit arch (apart from one that sports CMPXCHG8 or similar). > /* > - * load a scatterlist with a potentially split-page buffer > + * load a scatterlist > */ > -static void rxkad_sg_set_buf2(struct scatterlist sg[2], > +static void rxkad_sg_set_buf2(struct scatterlist sg[1], > void *buf, size_t buflen) > { > - int nsg = 1; > - > - sg_init_table(sg, 2); > - > + sg_init_table(sg, 1); > sg_set_buf(&sg[0], buf, buflen); > - if (sg[0].offset + buflen > PAGE_SIZE) { > - /* the buffer was split over two pages */ > - sg[0].length = PAGE_SIZE - sg[0].offset; > - sg_set_buf(&sg[1], buf + sg[0].length, buflen - sg[0].length); > - nsg++; > - } > - > - sg_mark_end(&sg[nsg - 1]); > - > - ASSERTCMP(sg[0].length + sg[1].length, ==, buflen); > } This should be a separate patch. David