From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753147AbdI1NHB (ORCPT ); Thu, 28 Sep 2017 09:07:01 -0400 Received: from mail-he1eur01on0126.outbound.protection.outlook.com ([104.47.0.126]:6416 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751906AbdI1NG5 (ORCPT ); Thu, 28 Sep 2017 09:06:57 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=aryabinin@virtuozzo.com; Subject: Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN To: Arnd Bergmann 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" References: <20170922212930.620249-1-arnd@arndb.de> <20170922212930.620249-5-arnd@arndb.de> <063D6719AE5E284EB5DD2968C1650D6DD007F521@AcuExch.aculab.com> From: Andrey Ryabinin Message-ID: <2631e8a6-03f2-69ea-d889-afd9a345e7ef@virtuozzo.com> Date: Thu, 28 Sep 2017 16:09:46 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: AM5PR0701CA0006.eurprd07.prod.outlook.com (2603:10a6:203:51::16) To DB6PR08MB2821.eurprd08.prod.outlook.com (2603:10a6:6:1d::24) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: fddde0e1-ce09-4afb-fe9a-08d50671c80a X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254152)(2017052603199)(201703131423075)(201703031133081)(201702281549075);SRVR:DB6PR08MB2821; X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2821;3:BtOuym7JTH7xvaS9F/bIrE760OvBFxVGFiwfIPhEM9fbcmS9QODJEjsaeC+HGNR6EqC+lcn1kmdVzhZhq75w5yPQ6e6ZtD/mU3dyaOhJylVoKXpShOOJ7p3JJRBNYm0qlIcT5bdwAogScX0ii/XQwk1H+YCHEaVKX2pevLVC4evC6tf/o3OMNpIhscs+rzERw4uZhSn3ajh4PRwEip+tV4Lmwy8SPg9DKUSwwka/AAtZ+xY1Vzv7bsoP/VW7+RKJ;25:2IDHNdwwDO35GZtYycmwyq0mlL3RSWx7PvZuVs4JotpWwe9zT7vTWmuss13x3/tEA3i1BG81OxT0nBOYWsYvk703nNmuiwBIL/gDZZhpVF92dhBKx+/stEykn4r/vsYxhu89mS4IGLSMKlq0g2fn0biBx6Ycd1k965N40ybZbSgk+AWhpdJ7xqDsWubgLDoOKpwu0u6I6Aajpy8L6gR1epEpK0CnTDvGulWu+fPrVSygTjWGDallsgExztcNr9sCZLlCbiuWgkNkYvHejRVtEXcUdhHWPrhJNfSawHGzgDH1EF8aFMRzsyMJ58fa8vqGgU/fx77pAJmKm+w0RKVLRA==;31:tGD4MHjJHvyrf+rv85HaV/96cnPObDqZtdBKCb19j6p6dyEk+M25+Qs9rEkmEOvT5PULhSJWbKYcca11nU+2+EUqDnt3W/GzoTgd0LFbTLyjCuoIEjf1C5Kl8RAYW0RPpX29CAJP0xIBcjTO9guMAzc3AnujxIy5Kdb1zzmj0hvKS4iPHlkEAIx7EDwoFpStvgStqSLG0cFaIQzZYaNS6kMhQSI+q46V40PFJFzOj5Q= X-MS-TrafficTypeDiagnostic: DB6PR08MB2821: X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2821;20:7BHYeSlYll01lx/LhGwNtDuU6D55luxxpLTmodNIO9Bav0sHsTE/Pggobktj4jUObBCNiBsg6yvT8RcrThtC/whRt9WFIGKd9GZkYytM86ymbRTFEtts0FjTtgMC9HeKqSQ3tvcUEyZbpAITHz76YIUnf/BHChZ89ej4fHmP58WnlRg+v3+HzeLUn9PdBdd7XHgrA3IMoolUSoJGKO7tPvX3/837p86H6tQUE0sTmTbruHqTlnkHH3U5wBrm7TOHbrF+E1lvHlTncLlJFmqLcmMMTPraruvHRh9KXYaXx//IHumnoTkTWYOYlNEc1r/oBh6s+1GYH3kJrBHghAd1rmy3G0PR/1qqDZcIRUBa0OzBd4bJkg8lhNKxK74j2JnJiVagQULjjaFthifeApGGyQHCSjajikmaPuikBrR7Gb0=;4:9R5xMUBOIHetdL3CobUgdEL4p/ZgeUm35GnUdFRLp+V/RfHrOn8lhsqyA8WucoTLDoNWxrA3Hn7/wFnysBxI9m8zksoysssz1PpGtSqAhQeXBT06jP5P72GCBfAv2FyMtYdTU65/G/LlPj/6xrVcK5HRe+a5QH4Crf4ZTaToM3Bvx4f6Lz2y1nfrINPVO0aPhPPHMZ1cVIuhoUK8rGhtOLmOJKNB2A7h0BuaRweHf4qCnbShVAcvDp899JONwSnF X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(100000703101)(100105400095)(6041248)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123558100)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:DB6PR08MB2821;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:DB6PR08MB2821; X-Forefront-PRVS: 0444EB1997 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6009001)(6049001)(346002)(376002)(39830400002)(199003)(24454002)(189002)(377454003)(316002)(478600001)(93886005)(2906002)(101416001)(50986999)(86362001)(229853002)(16526017)(16576012)(77096006)(6486002)(189998001)(53936002)(4326008)(7736002)(6306002)(83506001)(31686004)(50466002)(66066001)(76176999)(305945005)(68736007)(64126003)(54356999)(33646002)(65956001)(8676002)(65826007)(230700001)(65806001)(54906003)(97736004)(47776003)(7416002)(53546010)(966005)(6116002)(58126008)(5660300001)(25786009)(105586002)(106356001)(36756003)(3846002)(31696002)(81156014)(6246003)(81166006)(6916009)(23676002)(8936002)(2950100002);DIR:OUT;SFP:1102;SCL:1;SRVR:DB6PR08MB2821;H:[172.16.25.12];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjZQUjA4TUIyODIxOzIzOlQ4cTJ5T0ZacFB0Y0hwcDZ1R1puaUxnQlJM?= =?utf-8?B?SWJ2aDJyUmF0YnR3QkNmUGxtaVNvb3N4c0VkU3k4NXhYVVNIUm5TdTJMTjRM?= =?utf-8?B?ZkRtdXRxRFJZZVdsa1NCMm1KRXNZSWZrSmhMTWNWc09YbDJMZ1BkZmNBZm15?= =?utf-8?B?RG9HQ0hFOVlSK2tWRWpXTGl3Nm8wSmlPWmNLNTI1N0Q4RjZvMkozZHpZeXE1?= =?utf-8?B?YURqa3VLWlRuRTVMWjBpdWF4NnNodjhaRWFPa1NDVEJGbGR4MGNxbVU5Wk9o?= =?utf-8?B?aTdaTm01elFydy94RjBGOVovRnpLaWp3RHFKSmlFUnVPTjcvb1dwTEJHTXA5?= =?utf-8?B?R09URE9pSklxVndsV3BFVDhwU2FHQVdtU3FZVlNaaVdpWUhHTUYraXJ3bnJY?= =?utf-8?B?Q0lGazVZVG1sUm9sUG1pRklrQ0kzRWF5cEt6NGhwQm1LNXo0MnVpZk1EZFcx?= =?utf-8?B?cm8xQWx0UWxjeXUwZjA5cWhCRlhZaHRpdVdFSzl6SVp2eEljWHZLZ25CbnFX?= =?utf-8?B?eDY3Nk9BMUxtVlhucDlxQW9lczg5cHhZYUxHbGhib0JYWkIyT1hvbUNQZkNC?= =?utf-8?B?YlZ5VHpRK1NLYVkwWWNEWDVPVE9zQUxSV1dONmNLTVBsWHpiZGZvcllTVDd3?= =?utf-8?B?dDVoKytSOU1CQzArVFJkVUpYcWVSa2dqSnFiNVZUSXdEWTh4eUlLL0MzY3RT?= =?utf-8?B?SSswbEVoRlJFUDNnUEV5NDU3WGN2T3dGb0tHeVZXUGdJMUozU0cybEN6bE9j?= =?utf-8?B?OTdPWGFkMlEwNGxKalhrV25kemVFaXA5ZnVjd0FDc3pmMUhsa2o4eFBzRjNE?= =?utf-8?B?eWJLeE01STRNU0QrUDJMakpEMzI3dUhSZmY5Zzd1T29JTEs2VlNlY0hrQmpl?= =?utf-8?B?TkR3TFBsRDlMQmxiazhIR2NMNVdEWkhSczQ4dTU5QUFhZTBoV1ZwbWVsaUZv?= =?utf-8?B?M2VtdWtpbFMyMC9mMGx4MGE0OExLTzlXcnpxNzR6M3N6NURaS3lIdVZ5RUlu?= =?utf-8?B?ZFVTdm8rL0szV1VGUUp0MXdCeEE3N1ZucCtCUzJEU1NDa1IxUkI5SE9iSitS?= =?utf-8?B?eEJmZTRtYjJRWnZWWmE1U3hjQjYwZERoVnRBNFZOV0FRbkdlN2hhNCtKekYr?= =?utf-8?B?ZlEyZ1kySiswM2liRnE1OHBQNjBXcjU4bWdvTkZRSzkvRUV5V2xmNnZuVERz?= =?utf-8?B?dHVBYm05cVNZNFVKMmVQK01IeWxMK09uUjZ5SlpTcm85d0FyVkN0Y093TUpz?= =?utf-8?B?VWZLbGlHN3JPUnMzOEpqY1k1WmNZNDl2aDc4ZlVYTXI0Wm5PZ1Z1YTJlaDBV?= =?utf-8?B?TW1MNDJ3b0E1aGZxZmtiWVVQYmVMbHU1UjFjN09BVFV3ZElRK3VudTgrV2cv?= =?utf-8?B?QUZ1eTBXd1IwUmFDNGJ5ViswNzdndVdtZjBUY08xRVo3MEs4WnlwRE05SFd5?= =?utf-8?B?MXMwZkFEMWFITTdqODhyUDVCaFN4eGpURDBXVEhVUWg2dlQ4OGQ2bi94T0tR?= =?utf-8?B?ZkNJL29iVnpUNTRUcXZqaEdKdllSanljWitYTWl1NUc3V25DNDJJRmZPS2tK?= =?utf-8?B?Y2QwNWh2UG8yWUNMRURCQ3FrSm1keUU1dzdMMHEvTVJ5YkF3ZFU3S3o5WkZr?= =?utf-8?B?SXI1Yjh0MVFSNm1rWEZrenkxMGkxMmlmVldYd0FZOG5ITVJFR1RZbUJGTHdJ?= =?utf-8?B?dW9HRXNyYnA3TWFOaGNZVHZuTTZsQ0hXNUtUQldoQlpBOFFoYXFqVjVBZnVz?= =?utf-8?B?bEpwRWdCK3NQRUZKdGlVaHpCVm9ZSFNIenJQdm43WjhCZVZKdE1lTnp0Tm1l?= =?utf-8?B?QUh2cXJwRDdWYWMvQmtFOFg1alUxRUltaDI5TmxudG55U2NTMVpZNGhKaVRl?= =?utf-8?B?UnpxN3pEMERjTFZ3R3VaaUc1RmhMZE9Oc2U1bk9Qb29Id3k4WTMwNTdTY090?= =?utf-8?Q?59jRR/PR53jaUKrxQgi3dpRcPs9Xwc=3D?= X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2821;6:boKP9cG1kf3FKiOsibCX1thzTX2jCKxs5Ourl2EN1b6ljlZGgwKmyUvuM6ADIYQ8xRp4Bybj82W4DfcC5YbMIHVSzQKNq9+83MtJhru6jsx2AvGzedYWTBPYskT+x8rOnTBNn/6bRC8SpMm8Boih8zxuXjPJohmrrrUD3GKfHJlvrfVEpEiXZfAiTcLU88jwq5T/VUPIyxIt63qqE7IB3PyKtGQwnNQj8FA7D2yAwsDx5lIIVjXFJn6jQS0+DLEx0hLulfgMVPlR0FDqBzggxkJXQdcSDdZBKHd568lcdf+2GKiio5BSKp6cszddAD7vqN8+Tit/QFPO5IxG6LbIgw==;5:K0V6DoY86dPMzIOW6KieJsaobMdmraCqf9RRUVb0FMEqH8SmZMe8OnataGXjhigN998PhY1ZYtyrLZWrBfHSt2u2P9uDSPk4P9Indyc53S3PgONB69JBEYQB6HOVRWbN11KWMCjl0SlcdoqbDWRX9w==;24:+PMIUB8VAQ0yjo4TkkeKRzyJsOp4gaxx/iQjAwxv2Hiyn/+YheQzTVKmlJnAWMMSTmShA1Sl7qbJiT741+f9WvpMkmJ1BCpuKu8bZkpuHOk=;7:3l7pY/DImvhYHnwei6F8UNKCx4mSh41nvgsUHmkt9j9l7URX5HTQuGZAd5sTtG1cJkXpfV3Fzdsf98rj5VBe8Go+T5mwraTMsXQlJODubQ+ZAiogYLIuDMRYjrS28l3k0x6WMWs+li6t+9Qhhkd150bM4eddCebq8isiR71fy4NNRhcTQihg9FIjcayfUzmLTB5966Jc43UzyUbHErsWw6HlZiKuDbBOfByN5p2IOMs= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2821;20:zfN8PSPVFABenD4ywa0YDD/7llIjtLPm3kCdj+HQcuWPy+WKNE4iBLOPbqmy/w6amCDLhW2DFtoIThI8OTdT7d07NCsv3FsC8Oj6N+EVkQQYGz9ta4PXLRIH02Czxuw3cIaZjhglRozozs56mmuLUM+oNWi1lQt024q9CRt1xu4= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2017 13:06:48.1434 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2821 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/27/2017 04:26 PM, Arnd Bergmann wrote: > 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: > > --- 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 > 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? > At first I thought that this somehow might be related to __asan_handle_no_return(). GCC calls it before noreturn function. So I made patch to remove generation of these calls (we don't need them in the kernel anyway) but it didn't help. It must be something else than. >>> 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? Sure. > 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 >