linux-kernel - Re: [PATCH] em28xx: split up em28xx_dvb

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 15 Dec 2017 10:34:35 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Kevin Cheng <kcheng@...il.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Antti Palosaari <crope@....fi>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] em28xx: split up em28xx_dvb_init to reduce stack size

On Thu, Dec 14, 2017 at 6:10 PM, Mauro Carvalho Chehab
<mchehab@...nel.org> wrote:
> Em Mon, 11 Dec 2017 13:05:02 +0100
> Arnd Bergmann <arnd@...db.de> escreveu:
>
>> With CONFIG_KASAN, the init function uses a large amount of kernel stack:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
>>
>> Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
>>
>> By splitting out each part of the switch/case statement that has its own local
>> variables into a separate function, no single one of them uses more than 500 bytes,
>> and with a noinline_for_stack annotation we can ensure that they are not merged
>> back together.
>
> The right fix here is really to find a way to simplify the new method
> of binding I2C devices by using some ancillary routines.
>
> I'll keep this patch on my queue for now, but my plan is to try to solve
> this issue instead of applying it, maybe on the next weeks (as the
> volume of patches reduce due to end of year vacations and Seasons).

That's ok, thanks. We have a workaround in linux-mm that partially solves this
problem to the point where the stack size goes down to 1600 bytes with KASAN,
that by itself would be sufficient to let us enable CONFIG_FRAME_WARN
again for 64-bit platforms with the default 2048 byte warning limit. I reposted
that patch mostly since I want to lower the frame sizes further so we can
reduce the warning limit to 1280 bytes for 64-bit architectures in the future.
There around 10 patches needed for that, and they mostly seem to address
actual issues, so I'd like them to get addressed eventually and set the limit
low enough that the warnings we get on 64-bit are more useful again.

However, could you please revisit two other patches:

https://patchwork.linuxtv.org/patch/45716/ dvb-frontends: fix i2c
access helpers for KASAN
https://patchwork.linuxtv.org/patch/45709/ r820t: fix r820t_write_reg for KASAN

These are currently the ones I'm most interested in getting merged
into v4.15 and LTS kernels for the stack size reduction, since they
are blocking the patch that enables CONFIG_FRAME_WARN for
allmodconfig.

Thanks,

       Arnd