From patchwork Thu Dec 5 11:24:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 296894 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 914482C0040 for ; Thu, 5 Dec 2013 22:26:06 +1100 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VoX4D-0003bU-MH; Thu, 05 Dec 2013 11:26:01 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VoX2w-0002nb-Dy for kernel-team@lists.ubuntu.com; Thu, 05 Dec 2013 11:24:42 +0000 Received: from bl20-223-32.dsl.telepac.pt ([2.81.223.32] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1VoX2v-0007EE-Po; Thu, 05 Dec 2013 11:24:42 +0000 From: Luis Henriques To: Mauro Carvalho Chehab Subject: [3.11.y.z extended stable] Patch "[media] tuners: Don't use dynamic static allocation" has been added to staging queue Date: Thu, 5 Dec 2013 11:24:40 +0000 Message-Id: <1386242680-2056-1-git-send-email-luis.henriques@canonical.com> X-Mailer: git-send-email 1.8.3.2 X-Extended-Stable: 3.11 Cc: kernel-team@lists.ubuntu.com, Antti Palosaari , Hans Verkuil X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com This is a note to let you know that I have just added a patch titled [media] tuners: Don't use dynamic static allocation to the linux-3.11.y-queue branch of the 3.11.y.z extended stable tree which can be found at: http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.11.y-queue If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 3.11.y.z tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Luis ------ From c907bc4ecaf93b5941de077ac7591ab84c92bc64 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sat, 2 Nov 2013 06:07:42 -0300 Subject: [media] tuners: Don't use dynamic static allocation commit f1baab870f6e93b668af7b34d6f6ba49f1b0e982 upstream. Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/tuners/e4000.c:50:1: warning: 'e4000_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/e4000.c:83:1: warning: 'e4000_rd_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:66:1: warning: 'fc2580_wr_regs.constprop.1' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:98:1: warning: 'fc2580_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:57:1: warning: 'tda18212_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:90:1: warning: 'tda18212_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:60:1: warning: 'tda18218_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:92:1: warning: 'tda18218_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Hans Verkuil Reviewed-by: Antti Palosaari Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Luis Henriques --- drivers/media/tuners/e4000.c | 25 +++++++++++++++++++++---- drivers/media/tuners/fc2580.c | 25 +++++++++++++++++++++---- drivers/media/tuners/tda18212.c | 25 +++++++++++++++++++++---- drivers/media/tuners/tda18218.c | 23 ++++++++++++++++++++--- 4 files changed, 83 insertions(+), 15 deletions(-) -- 1.8.3.2 diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c index 1b33ed3..c4ce450 100644 --- a/drivers/media/tuners/e4000.c +++ b/drivers/media/tuners/e4000.c @@ -20,20 +20,30 @@ #include "e4000_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* write multiple registers */ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, .flags = 0, - .len = sizeof(buf), + .len = 1 + len, .buf = buf, } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -52,7 +62,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_addr, @@ -62,11 +72,18 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) }, { .addr = priv->cfg->i2c_addr, .flags = I2C_M_RD, - .len = sizeof(buf), + .len = len, .buf = buf, } }; + if (len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c rd reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, buf, len); diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 81f38aa..3aecaf4 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -20,6 +20,9 @@ #include "fc2580_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* * TODO: * I2C write and read works only for one single register. Multiple registers @@ -41,16 +44,23 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, .flags = 0, - .len = sizeof(buf), + .len = 1 + len, .buf = buf, } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -69,7 +79,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) static int fc2580_rd_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_addr, @@ -79,11 +89,18 @@ static int fc2580_rd_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) }, { .addr = priv->cfg->i2c_addr, .flags = I2C_M_RD, - .len = sizeof(buf), + .len = len, .buf = buf, } }; + if (len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c rd reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, buf, len); diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c index e4a84ee..abe256e 100644 --- a/drivers/media/tuners/tda18212.c +++ b/drivers/media/tuners/tda18212.c @@ -20,6 +20,9 @@ #include "tda18212.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct tda18212_priv { struct tda18212_config *cfg; struct i2c_adapter *i2c; @@ -32,16 +35,23 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len+1]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_address, .flags = 0, - .len = sizeof(buf), + .len = 1 + len, .buf = buf, } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -61,7 +71,7 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_address, @@ -71,11 +81,18 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, }, { .addr = priv->cfg->i2c_address, .flags = I2C_M_RD, - .len = sizeof(buf), + .len = len, .buf = buf, } }; + if (len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c rd reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, buf, len); diff --git a/drivers/media/tuners/tda18218.c b/drivers/media/tuners/tda18218.c index 2d31aeb..9300e93 100644 --- a/drivers/media/tuners/tda18218.c +++ b/drivers/media/tuners/tda18218.c @@ -20,11 +20,14 @@ #include "tda18218_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* write multiple registers */ static int tda18218_wr_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len) { int ret = 0, len2, remaining; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_address, @@ -33,6 +36,13 @@ static int tda18218_wr_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len) } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + for (remaining = len; remaining > 0; remaining -= (priv->cfg->i2c_wr_max - 1)) { len2 = remaining; @@ -63,7 +73,7 @@ static int tda18218_wr_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len) static int tda18218_rd_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len) { int ret; - u8 buf[reg+len]; /* we must start read always from reg 0x00 */ + u8 buf[MAX_XFER_SIZE]; /* we must start read always from reg 0x00 */ struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_address, @@ -73,11 +83,18 @@ static int tda18218_rd_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len) }, { .addr = priv->cfg->i2c_address, .flags = I2C_M_RD, - .len = sizeof(buf), + .len = reg + len, .buf = buf, } }; + if (reg + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, &buf[reg], len);