lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXu5jJJDF-3E_rt-16B-0CwjcF+52LPNGefD-KP+qz1JwM=6g@mail.gmail.com> Date: Sun, 29 Apr 2018 07:31:15 -0700 From: Kees Cook <keescook@...omium.org> To: Boris Brezillon <boris.brezillon@...tlin.com> Cc: LKML <linux-kernel@...r.kernel.org>, David Woodhouse <dwmw2@...radead.org>, Brian Norris <computersforpeace@...il.com>, Marek Vasut <marek.vasut@...il.com>, Richard Weinberger <richard@....at>, Linux mtd <linux-mtd@...ts.infradead.org> Subject: Re: [PATCH] mtd: nftl: Remove VLA usage On Sun, Apr 29, 2018 at 2:16 AM, Boris Brezillon <boris.brezillon@...tlin.com> wrote: > Hi Kees, > > On Mon, 23 Apr 2018 13:35:00 -0700 > Kees Cook <keescook@...omium.org> wrote: > >> On the quest to remove all VLAs from the kernel[1] this changes the >> check_free_sectors() routine to use the same stack buffer for both >> data byte checks (SECTORSIZE) and oob byte checks (oobsize). Since >> these regions aren't needed at the same time, they don't need to be >> consecutively allocated. Additionally, while it's possible for oobsize >> to be large, it is unlikely to be larger than the actual SECTORSIZE. As >> such, remove the VLA, adjust offsets and add a sanity check to make sure >> we never get a pathological oobsize. >> >> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com >> >> Signed-off-by: Kees Cook <keescook@...omium.org> >> --- >> drivers/mtd/inftlmount.c | 11 ++++++++--- >> drivers/mtd/nftlmount.c | 11 ++++++++--- >> 2 files changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c >> index aab4f68bd36f..9cdae7f0fc2e 100644 >> --- a/drivers/mtd/inftlmount.c >> +++ b/drivers/mtd/inftlmount.c >> @@ -334,7 +334,7 @@ static int memcmpb(void *a, int c, int n) >> static int check_free_sectors(struct INFTLrecord *inftl, unsigned int address, >> int len, int check_oob) >> { >> - u8 buf[SECTORSIZE + inftl->mbd.mtd->oobsize]; >> + u8 buf[SECTORSIZE]; > > Could we instead move to dynamic allocation. I mean, SECTORSIZE is 512 > bytes, so only with this function we consume 1/16 of the stack. Not to > mention that some MTD drivers might want to do DMA on buffer passed by > the MTD user, and if the buffer is on the stack they'll have to use a > bounce buffer instead. Sure! I can rework it to do that. Is GFP_KERNEL okay for that, or does it need something else? -Kees -- Kees Cook Pixel Security
Powered by blists - more mailing lists