From patchwork Tue Dec 12 10:24:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 10106591 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 564EE602C2 for ; Tue, 12 Dec 2017 10:24:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3F45329B71 for ; Tue, 12 Dec 2017 10:24:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 33E4E29B75; Tue, 12 Dec 2017 10:24:42 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,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 CBBE929B71 for ; Tue, 12 Dec 2017 10:24:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752582AbdLLKY3 (ORCPT ); Tue, 12 Dec 2017 05:24:29 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:41652 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752576AbdLLKY2 (ORCPT ); Tue, 12 Dec 2017 05:24:28 -0500 Received: by mail-oi0-f65.google.com with SMTP id t78so13652455oie.8; Tue, 12 Dec 2017 02:24:27 -0800 (PST) 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=0mMKUgVkb0urh8F65GVPWRkVCborOb9glBD2KWE6NCk=; b=c1m/CmUbg02IeVivy1aSpHVMIRkDa0cNxliAglk34SdmkG/jCjy7HV2oQSZ3a/hGX+ cm7OQKNZMk83nGON8mB+aN4yVXgYG5ifvrhZIda183wK03mzdNSf29X3pyT2b4CYPXuI fTONqklNWabf4LPOODm5EixhErPvSuDz5hxq74JUt2cRGPbWbMLXnORb0pdzcHsKT0wR SD6gCPoVwiKWqocQKjoE1ZdsVkjvEHqKUqiRpguiskjP9uolbEOP5etjHKvExWEuCOZ4 HT9HaRn3FVU+XiAaPE34rBzRmUQr/vdKdbPAtR9rmolbWO8MDRvtfVR8zq0nQnsM+RDD DJ2Q== 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=0mMKUgVkb0urh8F65GVPWRkVCborOb9glBD2KWE6NCk=; b=fMg/5/TcsaqAE+PBEwD4gtiteacjCl8NQvRY7lXA8IHnYejTz9sxc1fN5KaMYnir3F hPdAMjoqjYNPKoxifhy65K8g+z/goA3L4T2//x3lVevZNHf0WDRBFJtjrn3UqvIt2YN8 p9X5NFmxNcDpAXKb5bfg++hjr30bNthwGGcr7RR5cxKP9F7Gce0FD4JqXe4larjsSx/6 f3zN6igFD2PhQPGUQ1EIZAoVvnxY3pQmQJ5IeoVIle4+AhoeG8Mn89p3MSk9guHLBFE1 SCwTYxVVJwaF+Tr6sXca2wSz1iyoxC9Trn9pVRk5RB0+D6eRYt8Pb72P8j44EaHQZ5DT k9lg== X-Gm-Message-State: AKGB3mJdWhq+vHFOlmCAlBYVevy/lFUyNeZi8n0f1h7EvAElqplMYsF/ IB2YlfOH7FTCTJtzX5pCJ4EfL6h+UO/dLoF/IJg= X-Google-Smtp-Source: ACJfBosF6ny8GErDX4Tc7DdcnFfFfga7goQxFS8Dn/V8YOttINTt/W+r6FKyNcq+FUgEZdthBVosYx24XdNupizZl0o= X-Received: by 10.202.8.82 with SMTP id 79mr2577863oii.98.1513074267358; Tue, 12 Dec 2017 02:24:27 -0800 (PST) MIME-Version: 1.0 Received: by 10.157.43.3 with HTTP; Tue, 12 Dec 2017 02:24:26 -0800 (PST) In-Reply-To: References: <20171211120612.3775893-1-arnd@arndb.de> <1513020868.3036.0.camel@perches.com> From: Arnd Bergmann Date: Tue, 12 Dec 2017 11:24:26 +0100 X-Google-Sender-Auth: zfHP7CmjeQjDJPHHdZHiYnLU3rY Message-ID: Subject: Re: [PATCH] tuners: tda8290: reduce stack usage with kasan To: Michael Ira Krufky Cc: Joe Perches , Mauro Carvalho Chehab , linux-media , LKML Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Dec 11, 2017 at 10:17 PM, Michael Ira Krufky wrote: > On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches wrote: >> On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote: >>> With CONFIG_KASAN enabled, we get a relatively large stack frame in one function >>> >>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params': >>> drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes is larger than 1024 bytes [-Wframe-larger-than=] >>> >>> With CONFIG_KASAN_EXTRA this goes up to >>> >>> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params': >>> drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] >>> >>> We can significantly reduce this by marking local arrays as 'static const', and >>> this should result in better compiled code for everyone. >> [] >>> diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c >> [] >>> @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, int close) >>> { >>> struct tda8290_priv *priv = fe->analog_demod_priv; >>> >>> - unsigned char enable[2] = { 0x21, 0xC0 }; >>> - unsigned char disable[2] = { 0x21, 0x00 }; >>> + static unsigned char enable[2] = { 0x21, 0xC0 }; >>> + static unsigned char disable[2] = { 0x21, 0x00 }; >> >> Doesn't match commit message. >> >> static const or just static? >> >>> @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, int close) >>> { >>> struct tda8290_priv *priv = fe->analog_demod_priv; >>> >>> - unsigned char enable[2] = { 0x45, 0xc1 }; >>> - unsigned char disable[2] = { 0x46, 0x00 }; >>> - unsigned char buf[3] = { 0x45, 0x01, 0x00 }; >>> + static unsigned char enable[2] = { 0x45, 0xc1 }; >>> + static unsigned char disable[2] = { 0x46, 0x00 }; >> >> etc. >> >> > > > Joe is correct - they can be CONSTified. My bad -- a lot of the code I > wrote many years ago has this problem -- I wasn't so stack-conscious > back then. > > The bytes in `enable` / `disable` don't get changed, but they may be > copied to another byte array that does get changed. If would be best > to make these `static const` Right. This was an older patch of mine that I picked up again after running into a warning that I had been ignoring for a while, and I didn't double-check the message. I actually thought about marking them 'const' here before sending (without noticing the changelog text) and then ran into what must have led me to drop the 'const' originally: tuner_i2c_xfer_send() takes a non-const pointer. This can be fixed but it requires an ugly cast: int ret = i2c_transfer(props->adap, msg, 2); Should I submit it as a two-patch series with that added in, or update the changelog to not mention 'const' instead? Arnd diff --git a/drivers/media/tuners/tuner-i2c.h b/drivers/media/tuners/tuner-i2c.h index bda67a5a76f2..809466eec780 100644 --- a/drivers/media/tuners/tuner-i2c.h +++ b/drivers/media/tuners/tuner-i2c.h @@ -34,10 +34,10 @@ struct tuner_i2c_props { }; static inline int tuner_i2c_xfer_send(struct tuner_i2c_props *props, - unsigned char *buf, int len) + const unsigned char *buf, int len) { struct i2c_msg msg = { .addr = props->addr, .flags = 0, - .buf = buf, .len = len }; + .buf = (unsigned char *)buf, .len = len }; int ret = i2c_transfer(props->adap, &msg, 1); return (ret == 1) ? len : ret; @@ -54,11 +54,11 @@ static inline int tuner_i2c_xfer_recv(struct tuner_i2c_props *props, } static inline int tuner_i2c_xfer_send_recv(struct tuner_i2c_props *props, - unsigned char *obuf, int olen, + const unsigned char *obuf, int olen, unsigned char *ibuf, int ilen) { struct i2c_msg msg[2] = { { .addr = props->addr, .flags = 0, - .buf = obuf, .len = olen }, + .buf = (unsigned char *)obuf, .len = olen }, { .addr = props->addr, .flags = I2C_M_RD, .buf = ibuf, .len = ilen } };