From patchwork Wed Sep 27 13:26:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 9974045 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 42DD7602D6 for ; Wed, 27 Sep 2017 13:26:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 310912854A for ; Wed, 27 Sep 2017 13:26:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2583128702; Wed, 27 Sep 2017 13:26:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B1EBB2854A for ; Wed, 27 Sep 2017 13:26:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753166AbdI0N00 (ORCPT ); Wed, 27 Sep 2017 09:26:26 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:32937 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753090AbdI0N0W (ORCPT ); Wed, 27 Sep 2017 09:26:22 -0400 Received: by mail-oi0-f68.google.com with SMTP id z73so8748011oia.0; Wed, 27 Sep 2017 06:26:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=wUbgQ3NzqkySxz/FOtMJot84Y0Zv6VV3bATNf+isrgA=; b=ab/MeSsR3G7LdLH5aM/nTKlxCaCcsjdH0V5PYlx+iFRxRqrmQOeh1z9zND6g+x8vQG vmCso3P6yAjtx01nGMVCXE/JibETOtsd1V9Q8+zgtgMtTS6CeTS8o674VWs0ZoZQACdP wWygO5zd5wehDwMpwxBtdG9Hd1Ah/9628XD1nbqWS/NrS2jf2ovmaXZuirRTU71Aoehs 5bc8RsMZN07amXU4z35q5IY5RrV0ndpchQeWnjU1um62dhfjxqYO8IWgJKV04/EZ/aej doK82jmNxEE73a3a2HTKCr9sjgXrvqJId3Z1jMVxQ2jfMinoDPvMXa6uYaAdNM8RqH2G 1I9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=wUbgQ3NzqkySxz/FOtMJot84Y0Zv6VV3bATNf+isrgA=; b=Obah/E4UO+m9vTYWZvOjLPedkQatHshNxO3rAYDhxJubNc81lt+LiSrrhQvujNDzTA lGi6zHn5ztmJsuMLPAQRv/8FgF0iUjzhI8uY0McP0dZFM6kWgmxEtLBziFVwR+YvolyS pHSYclbmc7GrN4FHEPAobXOTDqXjkevtDpms9rHdDXtQPJf7tZpIdYLIyi9h+f4tXKKq ZWL+3MVn1gszJS3nbUxDKxwGSresEWZendwizDSuTSTN6lTLHW3IgfDgmPZATTl4xUhN C+RshoG2q/bQ8KO9Tg6Zd8MvnINv2GIZDwWOBiZXEUjPB0G6dnsgFzPkwlch520uLRmX 301w== X-Gm-Message-State: AMCzsaV8M076NtOVH8U+zZfyTgzf6eGkR8R8VfuibnQNVHE3O7C3xDTs lBmjVpga2qvp55M8yEtjA2nrxikQKYpS88Y4I7A= X-Google-Smtp-Source: AOwi7QDwQkhev8W/4muXAbIVp2ocXuZI45Pyay2k88a36O/aTVP4FykTjGdFmgHw0XIXszEAt0mv6sH9wXyjlRUR0bY= X-Received: by 10.157.17.6 with SMTP id g6mr876951ote.305.1506518781499; Wed, 27 Sep 2017 06:26:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.0.70 with HTTP; Wed, 27 Sep 2017 06:26:21 -0700 (PDT) In-Reply-To: References: <20170922212930.620249-1-arnd@arndb.de> <20170922212930.620249-5-arnd@arndb.de> <063D6719AE5E284EB5DD2968C1650D6DD007F521@AcuExch.aculab.com> From: Arnd Bergmann Date: Wed, 27 Sep 2017 06:26:21 -0700 X-Google-Sender-Auth: dDhfa6basSal96bnA8rFwdk5SU0 Message-ID: Subject: Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN To: Andrey Ryabinin Cc: David Laight , Mauro Carvalho Chehab , Jiri Pirko , Arend van Spriel , Kalle Valo , "David S. Miller" , Alexander Potapenko , Dmitry Vyukov , Masahiro Yamada , Michal Marek , Andrew Morton , Kees Cook , Geert Uytterhoeven , Greg Kroah-Hartman , "linux-media@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , "linux-wireless@vger.kernel.org" , "brcm80211-dev-list.pdl@broadcom.com" , "brcm80211-dev-list@cypress.com" , "kasan-dev@googlegroups.com" , "linux-kbuild@vger.kernel.org" , Jakub Jelinek , =?UTF-8?Q?Martin_Li=C5=A1ka?= , "stable@vger.kernel.org" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Sep 26, 2017 at 9:49 AM, Andrey Ryabinin wrote: > > > On 09/26/2017 09:47 AM, Arnd Bergmann wrote: >> On Mon, Sep 25, 2017 at 11:32 PM, Arnd Bergmann wrote: >> + ret = __builtin_strlen(q); > > > I think this is not correct. Fortified strlen called here on purpose. If sizeof q is known at compile time > and 'q' contains not-null fortified strlen() will panic. Ok, got it. >> if (size) { >> size_t len = (ret >= size) ? size - 1 : ret; >> if (__builtin_constant_p(len) && len >= p_size) >> >> The problem is apparently that the fortified strlcpy calls the fortified strlen, >> which in turn calls strnlen and that ends up calling the extern '__real_strnlen' >> that gcc cannot reduce to a constant expression for a constant input. > > > Per my observation, it's the code like this: > if () > fortify_panic(__func__); > > > somehow prevent gcc to merge several "struct i2c_board_info info;" into one stack slot. > With the hack bellow, stack usage reduced to ~1,6K: 1.6k is also what I see with my patch, or any other approach I tried that changes string.h. With the split up em28xx_dvb_init() function (and without changes to string.h), I got down to a few hundred bytes for the largest handler. > --- > include/linux/string.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/include/linux/string.h b/include/linux/string.h > index 54d21783e18d..9a96ff3ebf94 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -261,8 +261,6 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p) > if (p_size == (size_t)-1) > return __builtin_strlen(p); > ret = strnlen(p, p_size); > - if (p_size <= ret) > - fortify_panic(__func__); > return ret; > } > > @@ -271,8 +269,6 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen) > { > size_t p_size = __builtin_object_size(p, 0); > __kernel_size_t ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size); > - if (p_size <= ret && maxlen != ret) > - fortify_panic(__func__); > return ret; I've reduced it further to this change: size of object passed as 2nd parameter"); void __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter"); I don't immediately see why the __noreturn changes the behavior here, any idea? >> Not sure if that change is the best fix, but it seems to address the problem in >> this driver and probably leads to better code in other places as well. >> > > Probably it would be better to solve this on the strlcpy side, but I haven't found the way to do this right. > Alternative solutions: > > - use memcpy() instead of strlcpy(). All source strings are smaller than I2C_NAME_SIZE, so we could > do something like this - memcpy(info.type, "si2168", sizeof("si2168")); > Also this should be faster. This would be very similar to the patch I posted at the start of this thread to use strncpy(), right? I was hoping that changing strlcpy() here could also improve other users that might run into the same situation, but stay below the 2048-byte stack frame limit. > - Move code under different "case:" in the switch(dev->model) to the separate function should help as well. > But it might be harder to backport into stables. Agreed, I posted this in earlier versions of the patch series, see https://patchwork.kernel.org/patch/9601025/ The new patch was a result of me trying to come up with a less invasive version to make it easier to backport, since I would like to backport the last patch in the series that depends on all the earlier ones. Arnd --- a/include/linux/string.h +++ b/include/linux/string.h @@ -227,7 +227,7 @@ static inline const char *kbasename(const char *path) #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) #define __RENAME(x) __asm__(#x) -void fortify_panic(const char *name) __noreturn __cold; +void fortify_panic(const char *name) __cold; void __read_overflow(void) __compiletime_error("detected read beyond size of object passed as 1st parameter"); void __read_overflow2(void) __compiletime_error("detected read beyond