* [PATCH v3 01/29] [media] tda9887: remove an warning when compiling for alpha
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 02/29] [media] radio-shark: remove a warning when CONFIG_PM is not defined Mauro Carvalho Chehab
` (28 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
There's no need to zero the buffer, as if the routine gets an error,
rc will be different than one.
That fixes the following warning:
drivers/media/tuners/tda9887.c: In function 'tda9887_status':
drivers/media/tuners/tda9887.c:539:2: warning: value computed is not used [-Wunused-value]
While here, make fix the CodingStyle on this function.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/tuners/tda9887.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/tuners/tda9887.c b/drivers/media/tuners/tda9887.c
index 300005c535ba..9823248d743f 100644
--- a/drivers/media/tuners/tda9887.c
+++ b/drivers/media/tuners/tda9887.c
@@ -536,8 +536,8 @@ static int tda9887_status(struct dvb_frontend *fe)
unsigned char buf[1];
int rc;
- memset(buf,0,sizeof(buf));
- if (1 != (rc = tuner_i2c_xfer_recv(&priv->i2c_props,buf,1)))
+ rc = tuner_i2c_xfer_recv(&priv->i2c_props, buf, 1);
+ if (rc != 1)
tuner_info("i2c i/o error: rc == %d (should be 1)\n", rc);
dump_read_message(fe, buf);
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 02/29] [media] radio-shark: remove a warning when CONFIG_PM is not defined
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 01/29] [media] tda9887: remove an warning when compiling for alpha Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 03/29] [media] zoran: don't build it on alpha Mauro Carvalho Chehab
` (27 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
On alpha, allyesconfig doesn't have CONFIG_PM, and produces the following warnings:
drivers/media/radio/radio-shark.c:274:13: warning: 'shark_resume_leds' defined but not used [-Wunused-function]
drivers/media/radio/radio-shark2.c:240:13: warning: 'shark_resume_leds' defined but not used [-Wunused-function]
That's because those functions are used only at device resume.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/radio/radio-shark.c | 2 ++
drivers/media/radio/radio-shark2.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c
index b91477212413..3db8a8cfe1a8 100644
--- a/drivers/media/radio/radio-shark.c
+++ b/drivers/media/radio/radio-shark.c
@@ -271,6 +271,7 @@ static void shark_unregister_leds(struct shark_device *shark)
cancel_work_sync(&shark->led_work);
}
+#ifdef CONFIG_PM
static void shark_resume_leds(struct shark_device *shark)
{
if (test_bit(BLUE_IS_PULSE, &shark->brightness_new))
@@ -280,6 +281,7 @@ static void shark_resume_leds(struct shark_device *shark)
set_bit(RED_LED, &shark->brightness_new);
schedule_work(&shark->led_work);
}
+#endif
#else
static int shark_register_leds(struct shark_device *shark, struct device *dev)
{
diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c
index 9fb669721e66..d86d90dab8bf 100644
--- a/drivers/media/radio/radio-shark2.c
+++ b/drivers/media/radio/radio-shark2.c
@@ -237,6 +237,7 @@ static void shark_unregister_leds(struct shark_device *shark)
cancel_work_sync(&shark->led_work);
}
+#ifdef CONFIG_PM
static void shark_resume_leds(struct shark_device *shark)
{
int i;
@@ -246,6 +247,7 @@ static void shark_resume_leds(struct shark_device *shark)
schedule_work(&shark->led_work);
}
+#endif
#else
static int shark_register_leds(struct shark_device *shark, struct device *dev)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 03/29] [media] zoran: don't build it on alpha
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 01/29] [media] tda9887: remove an warning when compiling for alpha Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 02/29] [media] radio-shark: remove a warning when CONFIG_PM is not defined Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack Mauro Carvalho Chehab
` (26 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
This driver uses virt_to_bus() with is deprecated on Alpha:
drivers/media/pci/zoran/zoran_device.c: In function 'zr36057_set_vfe':
drivers/media/pci/zoran/zoran_device.c:451:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_device.c:453:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_device.c: In function 'zr36057_set_jpg':
drivers/media/pci/zoran/zoran_device.c:796:2: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_driver.c: In function 'v4l_fbuffer_alloc':
drivers/media/pci/zoran/zoran_driver.c:241:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_driver.c:245:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_driver.c: In function 'jpg_fbuffer_alloc':
drivers/media/pci/zoran/zoran_driver.c:334:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_driver.c:347:5: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
drivers/media/pci/zoran/zoran_driver.c:366:6: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations]
As we're not even sure if it works on Alpha, better to just disable its compilation there.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/pci/zoran/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
index 26ca8702e33f..39ec35bd21a5 100644
--- a/drivers/media/pci/zoran/Kconfig
+++ b/drivers/media/pci/zoran/Kconfig
@@ -1,6 +1,7 @@
config VIDEO_ZORAN
tristate "Zoran ZR36057/36067 Video For Linux"
depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
+ depends on !ALPHA
help
Say Y for support for MJPEG capture cards based on the Zoran
36057/36067 PCI controller chipset. This includes the Iomega
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (2 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 03/29] [media] zoran: don't build it on alpha Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-06 0:19 ` Andy Walls
2013-11-05 10:01 ` [PATCH v3 05/29] [media] tef6862: fix warning on avr32 arch Mauro Carvalho Chehab
` (25 subsequent siblings)
29 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
drivers/media/pci/cx18/cx18-driver.c: In function 'cx18_read_eeprom':
drivers/media/pci/cx18/cx18-driver.c:357:1: warning: the frame size of 1072 bytes is larger than 1024 bytes [-Wframe-larger-than=]
That happens because the routine allocates 256 bytes for an eeprom buffer, plus
the size of struct i2c_client, with is big.
Change the logic to dynamically allocate/deallocate space for struct i2c_client,
instead of using the stack.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/pci/cx18/cx18-driver.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
index ff7232023f56..87f5bcf29e90 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -324,23 +324,24 @@ static void cx18_eeprom_dump(struct cx18 *cx, unsigned char *eedata, int len)
/* Hauppauge card? get values from tveeprom */
void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
{
- struct i2c_client c;
+ struct i2c_client *c;
u8 eedata[256];
- memset(&c, 0, sizeof(c));
- strlcpy(c.name, "cx18 tveeprom tmp", sizeof(c.name));
- c.adapter = &cx->i2c_adap[0];
- c.addr = 0xA0 >> 1;
+ c = kzalloc(sizeof(*c), GFP_ATOMIC);
+
+ strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name));
+ c->adapter = &cx->i2c_adap[0];
+ c->addr = 0xa0 >> 1;
memset(tv, 0, sizeof(*tv));
- if (tveeprom_read(&c, eedata, sizeof(eedata)))
- return;
+ if (tveeprom_read(c, eedata, sizeof(eedata)))
+ goto ret;
switch (cx->card->type) {
case CX18_CARD_HVR_1600_ESMT:
case CX18_CARD_HVR_1600_SAMSUNG:
case CX18_CARD_HVR_1600_S5H1411:
- tveeprom_hauppauge_analog(&c, tv, eedata);
+ tveeprom_hauppauge_analog(c, tv, eedata);
break;
case CX18_CARD_YUAN_MPC718:
case CX18_CARD_GOTVIEW_PCI_DVD3:
@@ -354,6 +355,9 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
cx18_eeprom_dump(cx, eedata, sizeof(eedata));
break;
}
+
+ret:
+ kfree(c);
}
static void cx18_process_eeprom(struct cx18 *cx)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack
2013-11-05 10:01 ` [PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack Mauro Carvalho Chehab
@ 2013-11-06 0:19 ` Andy Walls
0 siblings, 0 replies; 39+ messages in thread
From: Andy Walls @ 2013-11-06 0:19 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
On Tue, 2013-11-05 at 08:01 -0200, Mauro Carvalho Chehab wrote:
> drivers/media/pci/cx18/cx18-driver.c: In function 'cx18_read_eeprom':
> drivers/media/pci/cx18/cx18-driver.c:357:1: warning: the frame size of 1072 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> That happens because the routine allocates 256 bytes for an eeprom buffer, plus
> the size of struct i2c_client, with is big.
> Change the logic to dynamically allocate/deallocate space for struct i2c_client,
> instead of using the stack.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
> drivers/media/pci/cx18/cx18-driver.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
> index ff7232023f56..87f5bcf29e90 100644
> --- a/drivers/media/pci/cx18/cx18-driver.c
> +++ b/drivers/media/pci/cx18/cx18-driver.c
> @@ -324,23 +324,24 @@ static void cx18_eeprom_dump(struct cx18 *cx, unsigned char *eedata, int len)
> /* Hauppauge card? get values from tveeprom */
> void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
> {
> - struct i2c_client c;
> + struct i2c_client *c;
> u8 eedata[256];
>
> - memset(&c, 0, sizeof(c));
> - strlcpy(c.name, "cx18 tveeprom tmp", sizeof(c.name));
> - c.adapter = &cx->i2c_adap[0];
> - c.addr = 0xA0 >> 1;
> + c = kzalloc(sizeof(*c), GFP_ATOMIC);
Hi Mauro,
GFP_ATOMIC seems overly strict, as this function is not in called in an
atomic context AFAIK.
Maybe use GFP_TEMPORARY or GFP_KERNEL.
Regards,
Andy
> +
> + strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name));
> + c->adapter = &cx->i2c_adap[0];
> + c->addr = 0xa0 >> 1;
>
> memset(tv, 0, sizeof(*tv));
> - if (tveeprom_read(&c, eedata, sizeof(eedata)))
> - return;
> + if (tveeprom_read(c, eedata, sizeof(eedata)))
> + goto ret;
>
> switch (cx->card->type) {
> case CX18_CARD_HVR_1600_ESMT:
> case CX18_CARD_HVR_1600_SAMSUNG:
> case CX18_CARD_HVR_1600_S5H1411:
> - tveeprom_hauppauge_analog(&c, tv, eedata);
> + tveeprom_hauppauge_analog(c, tv, eedata);
> break;
> case CX18_CARD_YUAN_MPC718:
> case CX18_CARD_GOTVIEW_PCI_DVD3:
> @@ -354,6 +355,9 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
> cx18_eeprom_dump(cx, eedata, sizeof(eedata));
> break;
> }
> +
> +ret:
> + kfree(c);
> }
>
> static void cx18_process_eeprom(struct cx18 *cx)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 05/29] [media] tef6862: fix warning on avr32 arch
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (3 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 06/29] [media] iguanair: simplify calculation of carrier delay cycles Mauro Carvalho Chehab
` (24 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
On avr32 arch, we get those warnings:
drivers/media/radio/tef6862.c:59:1: warning: "MODE_SHIFT" redefined
In file included from /devel/v4l/ktest-build/arch/avr32/include/asm/ptrace.h:11,
arch/avr32/include/uapi/asm/ptrace.h:41:1: warning: this is the location of the previous definition
Prefix MSA_ to the MSA register bitmap macros, to avoid reusing the same symbol.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/radio/tef6862.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/media/radio/tef6862.c b/drivers/media/radio/tef6862.c
index 06ac69245ca1..69e3245a58a0 100644
--- a/drivers/media/radio/tef6862.c
+++ b/drivers/media/radio/tef6862.c
@@ -48,15 +48,15 @@
#define WM_SUB_TEST 0xF
/* Different modes of the MSA register */
-#define MODE_BUFFER 0x0
-#define MODE_PRESET 0x1
-#define MODE_SEARCH 0x2
-#define MODE_AF_UPDATE 0x3
-#define MODE_JUMP 0x4
-#define MODE_CHECK 0x5
-#define MODE_LOAD 0x6
-#define MODE_END 0x7
-#define MODE_SHIFT 5
+#define MSA_MODE_BUFFER 0x0
+#define MSA_MODE_PRESET 0x1
+#define MSA_MODE_SEARCH 0x2
+#define MSA_MODE_AF_UPDATE 0x3
+#define MSA_MODE_JUMP 0x4
+#define MSA_MODE_CHECK 0x5
+#define MSA_MODE_LOAD 0x6
+#define MSA_MODE_END 0x7
+#define MSA_MODE_SHIFT 5
struct tef6862_state {
struct v4l2_subdev sd;
@@ -114,7 +114,7 @@ static int tef6862_s_frequency(struct v4l2_subdev *sd, const struct v4l2_frequen
clamp(freq, TEF6862_LO_FREQ, TEF6862_HI_FREQ);
pll = 1964 + ((freq - TEF6862_LO_FREQ) * 20) / FREQ_MUL;
- i2cmsg[0] = (MODE_PRESET << MODE_SHIFT) | WM_SUB_PLLM;
+ i2cmsg[0] = (MSA_MODE_PRESET << MSA_MODE_SHIFT) | WM_SUB_PLLM;
i2cmsg[1] = (pll >> 8) & 0xff;
i2cmsg[2] = pll & 0xff;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 06/29] [media] iguanair: simplify calculation of carrier delay cycles
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (4 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 05/29] [media] tef6862: fix warning on avr32 arch Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 07/29] [media] platform drivers: Fix build on frv arch Mauro Carvalho Chehab
` (23 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Sean Young
From: Sean Young <sean@mess.org>
Simplify the logic that calculates the carrier, and removes a warning
on avr32 arch:
drivers/media/rc/iguanair.c: In function 'iguanair_set_tx_carrier':
drivers/media/rc/iguanair.c:304: warning: 'sevens' may be used uninitialized in this function
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/rc/iguanair.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/media/rc/iguanair.c b/drivers/media/rc/iguanair.c
index 19632b1c2190..7f05e197680b 100644
--- a/drivers/media/rc/iguanair.c
+++ b/drivers/media/rc/iguanair.c
@@ -308,22 +308,12 @@ static int iguanair_set_tx_carrier(struct rc_dev *dev, uint32_t carrier)
cycles = DIV_ROUND_CLOSEST(24000000, carrier * 2) -
ir->cycle_overhead;
- /* make up the the remainer of 4-cycle blocks */
- switch (cycles & 3) {
- case 0:
- sevens = 0;
- break;
- case 1:
- sevens = 3;
- break;
- case 2:
- sevens = 2;
- break;
- case 3:
- sevens = 1;
- break;
- }
-
+ /*
+ * Calculate minimum number of 7 cycles needed so
+ * we are left with a multiple of 4; so we want to have
+ * (sevens * 7) & 3 == cycles & 3
+ */
+ sevens = (4 - cycles) & 3;
fours = (cycles - sevens * 7) / 4;
/* magic happens here */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 07/29] [media] platform drivers: Fix build on frv arch
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (5 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 06/29] [media] iguanair: simplify calculation of carrier delay cycles Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 08/29] [media] radio-si470x-i2c: fix a warning on ia64 Mauro Carvalho Chehab
` (22 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
On frv, the following errors happen:
drivers/media/platform/soc_camera/rcar_vin.c: In function 'rcar_vin_setup':
drivers/media/platform/soc_camera/rcar_vin.c:284:3: error: implicit declaration of function 'iowrite32' [-Werror=implicit-function-declaration]
drivers/media/platform/soc_camera/rcar_vin.c: In function 'rcar_vin_request_capture_stop':
drivers/media/platform/soc_camera/rcar_vin.c:353:2: error: implicit declaration of function 'ioread32' [-Werror=implicit-function-declaration]
This is because this driver forgot to include linux/io.h.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/platform/soc_camera/rcar_vin.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index b21f777f55e7..6866bb4fbebc 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -16,6 +16,7 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/platform_data/camera-rcar.h>
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 08/29] [media] radio-si470x-i2c: fix a warning on ia64
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (6 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 07/29] [media] platform drivers: Fix build on frv arch Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 09/29] [media] rc: Fir warnings on m68k arch Mauro Carvalho Chehab
` (21 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
on ia64, those warnings appear:
drivers/media/radio/si470x/radio-si470x-i2c.c:470:12: warning: 'si470x_i2c_suspend' defined but not used [-Wunused-function]
drivers/media/radio/si470x/radio-si470x-i2c.c:487:12: warning: 'si470x_i2c_resume' defined but not used [-Wunused-function]
They're caused because the PM logic uses this define:
#define SET_SYSTEM_SLEEP_PM_OPS()
With is only defined for CONFIG_PM_SLEEP.
So, change the logic there to test for CONFIG_PM_SLEEP, instead of
CONFIG_PM.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/radio/si470x/radio-si470x-i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index e5fc9acd0c4f..2a497c80c77f 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -463,7 +463,7 @@ static int si470x_i2c_remove(struct i2c_client *client)
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
/*
* si470x_i2c_suspend - suspend the device
*/
@@ -509,7 +509,7 @@ static struct i2c_driver si470x_i2c_driver = {
.driver = {
.name = "si470x",
.owner = THIS_MODULE,
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
.pm = &si470x_i2c_pm,
#endif
},
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 09/29] [media] rc: Fir warnings on m68k arch
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (7 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 08/29] [media] radio-si470x-i2c: fix a warning on ia64 Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 10/29] [media] uvc/lirc_serial: Fix some warnings on parisc arch Mauro Carvalho Chehab
` (20 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Fix the following warnings:
drivers/media/rc/fintek-cir.c: In function 'fintek_cr_write':
drivers/media/rc/fintek-cir.c:45:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c:46:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c: In function 'fintek_cr_read':
drivers/media/rc/fintek-cir.c:54:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c:55:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c: In function 'fintek_config_mode_enable':
drivers/media/rc/fintek-cir.c:80:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c:81:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/fintek-cir.c: In function 'fintek_config_mode_disable':
drivers/media/rc/fintek-cir.c:87:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c: In function 'nvt_cr_write':
drivers/media/rc/nuvoton-cir.c:45:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c:46:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c: In function 'nvt_cr_read':
drivers/media/rc/nuvoton-cir.c:52:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c:53:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c: In function 'nvt_efm_enable':
drivers/media/rc/nuvoton-cir.c:74:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c:75:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c: In function 'nvt_efm_disable':
drivers/media/rc/nuvoton-cir.c:81:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c: In function 'nvt_select_logical_dev':
drivers/media/rc/nuvoton-cir.c:91:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
drivers/media/rc/nuvoton-cir.c:92:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
Those are caused because the I/O port is u32, instead of u8.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/rc/fintek-cir.h | 4 ++--
drivers/media/rc/nuvoton-cir.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/rc/fintek-cir.h b/drivers/media/rc/fintek-cir.h
index 82516a1d39b0..b698f3d2ced9 100644
--- a/drivers/media/rc/fintek-cir.h
+++ b/drivers/media/rc/fintek-cir.h
@@ -76,8 +76,8 @@ struct fintek_dev {
} tx;
/* Config register index/data port pair */
- u8 cr_ip;
- u8 cr_dp;
+ u32 cr_ip;
+ u32 cr_dp;
/* hardware I/O settings */
unsigned long cir_addr;
diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h
index 7c3674ff5ea2..07e83108df0f 100644
--- a/drivers/media/rc/nuvoton-cir.h
+++ b/drivers/media/rc/nuvoton-cir.h
@@ -84,8 +84,8 @@ struct nvt_dev {
} tx;
/* EFER Config register index/data pair */
- u8 cr_efir;
- u8 cr_efdr;
+ u32 cr_efir;
+ u32 cr_efdr;
/* hardware I/O settings */
unsigned long cir_addr;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 10/29] [media] uvc/lirc_serial: Fix some warnings on parisc arch
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (8 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 09/29] [media] rc: Fir warnings on m68k arch Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 11/29] [media] s5h1420: Don't use dynamic static allocation Mauro Carvalho Chehab
` (19 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
On this arch, usec is not unsigned long. So, we need to typecast,
in order to remove those warnings:
drivers/media/usb/uvc/uvc_video.c: In function 'uvc_video_clock_update':
drivers/media/usb/uvc/uvc_video.c:678:2: warning: format '%lu' expects argument of type 'long unsigned int', but argument 9 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c: In function 'irq_handler':
drivers/staging/media/lirc/lirc_serial.c:707:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c:707:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c:719:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c:719:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c:728:6: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat]
drivers/staging/media/lirc/lirc_serial.c:728:6: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat]
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/uvc/uvc_video.c | 3 ++-
drivers/staging/media/lirc/lirc_serial.c | 9 ++++++---
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 3394c3432011..899cb6d1c4a4 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -680,7 +680,8 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
stream->dev->name,
sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
y, ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC,
- v4l2_buf->timestamp.tv_sec, v4l2_buf->timestamp.tv_usec,
+ v4l2_buf->timestamp.tv_sec,
+ (unsigned long)v4l2_buf->timestamp.tv_usec,
x1, first->host_sof, first->dev_sof,
x2, last->host_sof, last->dev_sof, y1, y2);
diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index af08e677b60f..7b3be2346b4b 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -707,7 +707,8 @@ static irqreturn_t irq_handler(int i, void *blah)
pr_warn("ignoring spike: %d %d %lx %lx %lx %lx\n",
dcd, sense,
tv.tv_sec, lasttv.tv_sec,
- tv.tv_usec, lasttv.tv_usec);
+ (unsigned long)tv.tv_usec,
+ (unsigned long)lasttv.tv_usec);
continue;
}
@@ -719,7 +720,8 @@ static irqreturn_t irq_handler(int i, void *blah)
pr_warn("%d %d %lx %lx %lx %lx\n",
dcd, sense,
tv.tv_sec, lasttv.tv_sec,
- tv.tv_usec, lasttv.tv_usec);
+ (unsigned long)tv.tv_usec,
+ (unsigned long)lasttv.tv_usec);
data = PULSE_MASK;
} else if (deltv > 15) {
data = PULSE_MASK; /* really long time */
@@ -728,7 +730,8 @@ static irqreturn_t irq_handler(int i, void *blah)
pr_warn("AIEEEE: %d %d %lx %lx %lx %lx\n",
dcd, sense,
tv.tv_sec, lasttv.tv_sec,
- tv.tv_usec, lasttv.tv_usec);
+ (unsigned long)tv.tv_usec,
+ (unsigned long)lasttv.tv_usec);
/*
* detecting pulse while this
* MUST be a space!
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 11/29] [media] s5h1420: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (9 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 10/29] [media] uvc/lirc_serial: Fix some warnings on parisc arch Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 12/29] [media] dvb-frontends: " Mauro Carvalho Chehab
` (18 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/s5h1420.c:851:1: warning: 's5h1420_tuner_i2c_tuner_xfer' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer.
In the specific case of this frontend, only ttpci uses it. The maximum
number of messages there is two, on I2C read operations. As the logic
can add an extra operation, change the size to 3.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/s5h1420.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/s5h1420.c b/drivers/media/dvb-frontends/s5h1420.c
index e2fec9ebf947..97c400a4297f 100644
--- a/drivers/media/dvb-frontends/s5h1420.c
+++ b/drivers/media/dvb-frontends/s5h1420.c
@@ -836,9 +836,16 @@ static u32 s5h1420_tuner_i2c_func(struct i2c_adapter *adapter)
static int s5h1420_tuner_i2c_tuner_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msg[], int num)
{
struct s5h1420_state *state = i2c_get_adapdata(i2c_adap);
- struct i2c_msg m[1 + num];
+ struct i2c_msg m[3];
u8 tx_open[2] = { CON_1, state->CON_1_val | 1 }; /* repeater stops once there was a stop condition */
+ if (1 + num > ARRAY_SIZE(m)) {
+ printk(KERN_WARNING
+ "%s: i2c xfer: num=%d is too big!\n",
+ KBUILD_MODNAME, num);
+ return -EOPNOTSUPP;
+ }
+
memset(m, 0, sizeof(struct i2c_msg) * (1 + num));
m[0].addr = state->config->demod_address;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 12/29] [media] dvb-frontends: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (10 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 11/29] [media] s5h1420: Don't use dynamic static allocation Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 13/29] " Mauro Carvalho Chehab
` (17 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/itd1000.c:69:1: warning: 'itd1000_write_regs.constprop.0' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/mt312.c:126:1: warning: 'mt312_write' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/nxt200x.c:111:1: warning: 'nxt200x_writebytes' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/stb6100.c:216:1: warning: 'stb6100_write_reg_range.constprop.3' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/stv6110.c:98:1: warning: 'stv6110_write_regs' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/stv6110x.c:85:1: warning: 'stv6110x_write_regs' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/tda18271c2dd.c:147:1: warning: 'WriteRegs' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/zl10039.c:119:1: warning: 'zl10039_write' 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 <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/bcm3510.c | 15 ++++++++++++++-
drivers/media/dvb-frontends/itd1000.c | 13 ++++++++++++-
drivers/media/dvb-frontends/mt312.c | 10 +++++++++-
drivers/media/dvb-frontends/nxt200x.c | 11 ++++++++++-
drivers/media/dvb-frontends/stb6100.c | 11 ++++++++++-
drivers/media/dvb-frontends/stv6110.c | 12 +++++++++++-
drivers/media/dvb-frontends/stv6110x.c | 13 ++++++++++++-
drivers/media/dvb-frontends/tda18271c2dd.c | 14 ++++++++++++--
drivers/media/dvb-frontends/zl10039.c | 12 +++++++++++-
9 files changed, 101 insertions(+), 10 deletions(-)
diff --git a/drivers/media/dvb-frontends/bcm3510.c b/drivers/media/dvb-frontends/bcm3510.c
index 1b77909c0c71..39a29dd29519 100644
--- a/drivers/media/dvb-frontends/bcm3510.c
+++ b/drivers/media/dvb-frontends/bcm3510.c
@@ -44,6 +44,9 @@
#include "bcm3510.h"
#include "bcm3510_priv.h"
+/* Max transfer size done by bcm3510_do_hab_cmd() function */
+#define MAX_XFER_SIZE 128
+
struct bcm3510_state {
struct i2c_adapter* i2c;
@@ -201,9 +204,19 @@ static int bcm3510_hab_send_request(struct bcm3510_state *st, u8 *buf, int len)
static int bcm3510_do_hab_cmd(struct bcm3510_state *st, u8 cmd, u8 msgid, u8 *obuf, u8 olen, u8 *ibuf, u8 ilen)
{
- u8 ob[olen+2],ib[ilen+2];
+ u8 ob[MAX_XFER_SIZE], ib[MAX_XFER_SIZE];
int ret = 0;
+ if (ilen + 2 > sizeof(ib)) {
+ deb_hab("do_hab_cmd: ilen=%d is too big!\n", ilen);
+ return -EINVAL;
+ }
+
+ if (olen + 2 > sizeof(ob)) {
+ deb_hab("do_hab_cmd: olen=%d is too big!\n", olen);
+ return -EINVAL;
+ }
+
ob[0] = cmd;
ob[1] = msgid;
memcpy(&ob[2],obuf,olen);
diff --git a/drivers/media/dvb-frontends/itd1000.c b/drivers/media/dvb-frontends/itd1000.c
index c1c3400b2173..cadcae4cff89 100644
--- a/drivers/media/dvb-frontends/itd1000.c
+++ b/drivers/media/dvb-frontends/itd1000.c
@@ -31,6 +31,9 @@
#include "itd1000.h"
#include "itd1000_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Turn on/off debugging (default:off).");
@@ -52,10 +55,18 @@ MODULE_PARM_DESC(debug, "Turn on/off debugging (default:off).");
/* don't write more than one byte with flexcop behind */
static int itd1000_write_regs(struct itd1000_state *state, u8 reg, u8 v[], u8 len)
{
- u8 buf[1+len];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = state->cfg->i2c_address, .flags = 0, .buf = buf, .len = len+1
};
+
+ if (1 + len > sizeof(buf)) {
+ printk(KERN_WARNING
+ "itd1000: i2c wr reg=%04x: len=%d is too big!\n",
+ reg, len);
+ return -EINVAL;
+ }
+
buf[0] = reg;
memcpy(&buf[1], v, len);
diff --git a/drivers/media/dvb-frontends/mt312.c b/drivers/media/dvb-frontends/mt312.c
index ec388c1d6913..a74ac0ddb833 100644
--- a/drivers/media/dvb-frontends/mt312.c
+++ b/drivers/media/dvb-frontends/mt312.c
@@ -36,6 +36,8 @@
#include "mt312_priv.h"
#include "mt312.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
struct mt312_state {
struct i2c_adapter *i2c;
@@ -96,9 +98,15 @@ static int mt312_write(struct mt312_state *state, const enum mt312_reg_addr reg,
const u8 *src, const size_t count)
{
int ret;
- u8 buf[count + 1];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg;
+ if (1 + count > sizeof(buf)) {
+ printk(KERN_WARNING
+ "mt312: write: len=%zd is too big!\n", count);
+ return -EINVAL;
+ }
+
if (debug) {
int i;
dprintk("W(%d):", reg & 0x7f);
diff --git a/drivers/media/dvb-frontends/nxt200x.c b/drivers/media/dvb-frontends/nxt200x.c
index 8e288940a61f..fbca9856313a 100644
--- a/drivers/media/dvb-frontends/nxt200x.c
+++ b/drivers/media/dvb-frontends/nxt200x.c
@@ -39,6 +39,9 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
#define NXT2002_DEFAULT_FIRMWARE "dvb-fe-nxt2002.fw"
#define NXT2004_DEFAULT_FIRMWARE "dvb-fe-nxt2004.fw"
#define CRC_CCIT_MASK 0x1021
@@ -95,10 +98,16 @@ static int i2c_readbytes(struct nxt200x_state *state, u8 addr, u8 *buf, u8 len)
static int nxt200x_writebytes (struct nxt200x_state* state, u8 reg,
const u8 *buf, u8 len)
{
- u8 buf2 [len+1];
+ u8 buf2[MAX_XFER_SIZE];
int err;
struct i2c_msg msg = { .addr = state->config->demod_address, .flags = 0, .buf = buf2, .len = len + 1 };
+ if (1 + len > sizeof(buf2)) {
+ pr_warn("%s: i2c wr reg=%04x: len=%d is too big!\n",
+ __func__, reg, len);
+ return -EINVAL;
+ }
+
buf2[0] = reg;
memcpy(&buf2[1], buf, len);
diff --git a/drivers/media/dvb-frontends/stb6100.c b/drivers/media/dvb-frontends/stb6100.c
index 45f9523f968f..cea175d19890 100644
--- a/drivers/media/dvb-frontends/stb6100.c
+++ b/drivers/media/dvb-frontends/stb6100.c
@@ -31,6 +31,8 @@
static unsigned int verbose;
module_param(verbose, int, 0644);
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
#define FE_ERROR 0
#define FE_NOTICE 1
@@ -183,7 +185,7 @@ static int stb6100_read_reg(struct stb6100_state *state, u8 reg)
static int stb6100_write_reg_range(struct stb6100_state *state, u8 buf[], int start, int len)
{
int rc;
- u8 cmdbuf[len + 1];
+ u8 cmdbuf[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = state->config->tuner_address,
.flags = 0,
@@ -191,6 +193,13 @@ static int stb6100_write_reg_range(struct stb6100_state *state, u8 buf[], int st
.len = len + 1
};
+ if (1 + len > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr: len=%d is too big!\n",
+ KBUILD_MODNAME, len);
+ return -EINVAL;
+ }
+
if (unlikely(start < 1 || start + len > STB6100_NUMREGS)) {
dprintk(verbose, FE_ERROR, 1, "Invalid register range %d:%d",
start, len);
diff --git a/drivers/media/dvb-frontends/stv6110.c b/drivers/media/dvb-frontends/stv6110.c
index 20b5fa92c53e..b1425830a24e 100644
--- a/drivers/media/dvb-frontends/stv6110.c
+++ b/drivers/media/dvb-frontends/stv6110.c
@@ -30,6 +30,9 @@
#include "stv6110.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static int debug;
struct stv6110_priv {
@@ -68,7 +71,7 @@ static int stv6110_write_regs(struct dvb_frontend *fe, u8 buf[],
{
struct stv6110_priv *priv = fe->tuner_priv;
int rc;
- u8 cmdbuf[len + 1];
+ u8 cmdbuf[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = priv->i2c_address,
.flags = 0,
@@ -78,6 +81,13 @@ static int stv6110_write_regs(struct dvb_frontend *fe, u8 buf[],
dprintk("%s\n", __func__);
+ if (1 + len > sizeof(cmdbuf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr: len=%d is too big!\n",
+ KBUILD_MODNAME, len);
+ return -EINVAL;
+ }
+
if (start + len > 8)
return -EINVAL;
diff --git a/drivers/media/dvb-frontends/stv6110x.c b/drivers/media/dvb-frontends/stv6110x.c
index f36cab12bdc7..e66154e5c1d7 100644
--- a/drivers/media/dvb-frontends/stv6110x.c
+++ b/drivers/media/dvb-frontends/stv6110x.c
@@ -32,6 +32,9 @@
#include "stv6110x.h"
#include "stv6110x_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static unsigned int verbose;
module_param(verbose, int, 0644);
MODULE_PARM_DESC(verbose, "Set Verbosity level");
@@ -61,7 +64,8 @@ static int stv6110x_write_regs(struct stv6110x_state *stv6110x, int start, u8 da
{
int ret;
const struct stv6110x_config *config = stv6110x->config;
- u8 buf[len + 1];
+ u8 buf[MAX_XFER_SIZE];
+
struct i2c_msg msg = {
.addr = config->addr,
.flags = 0,
@@ -69,6 +73,13 @@ static int stv6110x_write_regs(struct stv6110x_state *stv6110x, int start, u8 da
.len = len + 1
};
+ if (1 + len > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr: len=%d is too big!\n",
+ KBUILD_MODNAME, len);
+ return -EINVAL;
+ }
+
if (start + len > 8)
return -EINVAL;
diff --git a/drivers/media/dvb-frontends/tda18271c2dd.c b/drivers/media/dvb-frontends/tda18271c2dd.c
index d281f77d5c28..2c54586ac07f 100644
--- a/drivers/media/dvb-frontends/tda18271c2dd.c
+++ b/drivers/media/dvb-frontends/tda18271c2dd.c
@@ -34,6 +34,9 @@
#include "dvb_frontend.h"
#include "tda18271c2dd.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
struct SStandardParam {
s32 m_IFFrequency;
u32 m_BandWidth;
@@ -139,11 +142,18 @@ static int i2c_write(struct i2c_adapter *adap, u8 adr, u8 *data, int len)
static int WriteRegs(struct tda_state *state,
u8 SubAddr, u8 *Regs, u16 nRegs)
{
- u8 data[nRegs+1];
+ u8 data[MAX_XFER_SIZE];
+
+ if (1 + nRegs > sizeof(data)) {
+ printk(KERN_WARNING
+ "%s: i2c wr: len=%d is too big!\n",
+ KBUILD_MODNAME, nRegs);
+ return -EINVAL;
+ }
data[0] = SubAddr;
memcpy(data + 1, Regs, nRegs);
- return i2c_write(state->i2c, state->adr, data, nRegs+1);
+ return i2c_write(state->i2c, state->adr, data, nRegs + 1);
}
static int WriteReg(struct tda_state *state, u8 SubAddr, u8 Reg)
diff --git a/drivers/media/dvb-frontends/zl10039.c b/drivers/media/dvb-frontends/zl10039.c
index eff9c5fde50a..91b6b2e9b792 100644
--- a/drivers/media/dvb-frontends/zl10039.c
+++ b/drivers/media/dvb-frontends/zl10039.c
@@ -30,6 +30,9 @@
static int debug;
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
#define dprintk(args...) \
do { \
if (debug) \
@@ -98,7 +101,7 @@ static int zl10039_write(struct zl10039_state *state,
const enum zl10039_reg_addr reg, const u8 *src,
const size_t count)
{
- u8 buf[count + 1];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = state->i2c_addr,
.flags = 0,
@@ -106,6 +109,13 @@ static int zl10039_write(struct zl10039_state *state,
.len = count + 1,
};
+ if (1 + count > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr reg=%04x: len=%zd is too big!\n",
+ KBUILD_MODNAME, reg, count);
+ return -EINVAL;
+ }
+
dprintk("%s\n", __func__);
/* Write register address and data in one go */
buf[0] = reg;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 13/29] [media] dvb-frontends: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (11 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 12/29] [media] dvb-frontends: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 17:00 ` Antti Palosaari
2013-11-05 10:01 ` [PATCH v3 14/29] [media] stb0899_drv: " Mauro Carvalho Chehab
` (16 subsequent siblings)
29 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/af9013.c:77:1: warning: 'af9013_wr_regs_i2c' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/af9033.c:188:1: warning: 'af9033_wr_reg_val_tab' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/af9033.c:68:1: warning: 'af9033_wr_regs' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/cxd2820r_core.c:84:1: warning: 'cxd2820r_rd_regs_i2c.isra.1' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/rtl2830.c:56:1: warning: 'rtl2830_wr' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/rtl2832.c:187:1: warning: 'rtl2832_wr' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/tda10071.c:52:1: warning: 'tda10071_wr_regs' uses dynamic stack allocation [enabled by default]
drivers/media/dvb-frontends/tda10071.c:84:1: warning: 'tda10071_rd_regs' 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 <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/af9013.c | 12 +++++++++++-
drivers/media/dvb-frontends/af9033.c | 21 +++++++++++++++++++--
drivers/media/dvb-frontends/cxd2820r_core.c | 21 +++++++++++++++++++--
drivers/media/dvb-frontends/rtl2830.c | 12 +++++++++++-
drivers/media/dvb-frontends/rtl2832.c | 12 +++++++++++-
drivers/media/dvb-frontends/tda10071.c | 21 +++++++++++++++++++--
6 files changed, 90 insertions(+), 9 deletions(-)
diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
index a204f2828820..19ba66ad23fa 100644
--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -24,6 +24,9 @@
#include "af9013_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
struct af9013_state {
struct i2c_adapter *i2c;
struct dvb_frontend fe;
@@ -50,7 +53,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
const u8 *val, int len)
{
int ret;
- u8 buf[3+len];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[1] = {
{
.addr = priv->config.i2c_addr,
@@ -60,6 +63,13 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
}
};
+ if (3 + 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 >> 8) & 0xff;
buf[1] = (reg >> 0) & 0xff;
buf[2] = mbox;
diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c
index a777b4b944eb..11f1555e66dc 100644
--- a/drivers/media/dvb-frontends/af9033.c
+++ b/drivers/media/dvb-frontends/af9033.c
@@ -21,6 +21,9 @@
#include "af9033_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
struct af9033_state {
struct i2c_adapter *i2c;
struct dvb_frontend fe;
@@ -40,7 +43,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
int len)
{
int ret;
- u8 buf[3 + len];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[1] = {
{
.addr = state->cfg.i2c_addr,
@@ -50,6 +53,13 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
}
};
+ if (3 + len > sizeof(buf)) {
+ dev_warn(&state->i2c->dev,
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, len);
+ return -EINVAL;
+ }
+
buf[0] = (reg >> 16) & 0xff;
buf[1] = (reg >> 8) & 0xff;
buf[2] = (reg >> 0) & 0xff;
@@ -161,7 +171,14 @@ static int af9033_wr_reg_val_tab(struct af9033_state *state,
const struct reg_val *tab, int tab_len)
{
int ret, i, j;
- u8 buf[tab_len];
+ u8 buf[MAX_XFER_SIZE];
+
+ if (tab_len > sizeof(buf)) {
+ dev_warn(&state->i2c->dev,
+ "%s: i2c wr len=%d is too big!\n",
+ KBUILD_MODNAME, tab_len);
+ return -EINVAL;
+ }
dev_dbg(&state->i2c->dev, "%s: tab_len=%d\n", __func__, tab_len);
diff --git a/drivers/media/dvb-frontends/cxd2820r_core.c b/drivers/media/dvb-frontends/cxd2820r_core.c
index d9eeeb1dfa96..03930d5e9fea 100644
--- a/drivers/media/dvb-frontends/cxd2820r_core.c
+++ b/drivers/media/dvb-frontends/cxd2820r_core.c
@@ -21,12 +21,15 @@
#include "cxd2820r_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
/* write multiple registers */
static int cxd2820r_wr_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
u8 *val, int len)
{
int ret;
- u8 buf[len+1];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[1] = {
{
.addr = i2c,
@@ -36,6 +39,13 @@ static int cxd2820r_wr_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
}
};
+ 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);
@@ -55,7 +65,7 @@ static int cxd2820r_rd_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
u8 *val, int len)
{
int ret;
- u8 buf[len];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[2] = {
{
.addr = i2c,
@@ -70,6 +80,13 @@ static int cxd2820r_rd_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
}
};
+ if (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, len);
diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
index 362d26d11e82..b908800b390d 100644
--- a/drivers/media/dvb-frontends/rtl2830.c
+++ b/drivers/media/dvb-frontends/rtl2830.c
@@ -27,11 +27,14 @@
#include "rtl2830_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
/* write multiple hardware registers */
static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const 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,
@@ -41,6 +44,13 @@ static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const u8 *val, int 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;
+ }
+
buf[0] = reg;
memcpy(&buf[1], val, len);
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index a95dfe0a5ce3..cd1e6965ac11 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -22,6 +22,9 @@
#include "dvb_math.h"
#include <linux/bitops.h>
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
int rtl2832_debug;
module_param_named(debug, rtl2832_debug, int, 0644);
MODULE_PARM_DESC(debug, "Turn on/off frontend debugging (default:off).");
@@ -162,7 +165,7 @@ static const struct rtl2832_reg_entry registers[] = {
static int rtl2832_wr(struct rtl2832_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,
@@ -172,6 +175,13 @@ static int rtl2832_wr(struct rtl2832_priv *priv, u8 reg, u8 *val, int 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;
+ }
+
buf[0] = reg;
memcpy(&buf[1], val, len);
diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c
index e79749cfec81..1d8bc2ea4b10 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -20,6 +20,9 @@
#include "tda10071_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static struct dvb_frontend_ops tda10071_ops;
/* write multiple registers */
@@ -27,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_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.demod_i2c_addr,
@@ -37,6 +40,13 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
}
};
+ 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);
@@ -56,7 +66,7 @@ static int tda10071_rd_regs(struct tda10071_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.demod_i2c_addr,
@@ -71,6 +81,13 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
}
};
+ if (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, len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 13/29] [media] dvb-frontends: Don't use dynamic static allocation
2013-11-05 10:01 ` [PATCH v3 13/29] " Mauro Carvalho Chehab
@ 2013-11-05 17:00 ` Antti Palosaari
0 siblings, 0 replies; 39+ messages in thread
From: Antti Palosaari @ 2013-11-05 17:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Acked-by: Antti Palosaari <crope@iki.fi>
Reviewed-by: Antti Palosaari <crope@iki.fi>
Antti
On 05.11.2013 12:01, Mauro Carvalho Chehab wrote:
> Dynamic static allocation is evil, as Kernel stack is too low, and
> compilation complains about it on some archs:
> drivers/media/dvb-frontends/af9013.c:77:1: warning: 'af9013_wr_regs_i2c' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/af9033.c:188:1: warning: 'af9033_wr_reg_val_tab' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/af9033.c:68:1: warning: 'af9033_wr_regs' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/cxd2820r_core.c:84:1: warning: 'cxd2820r_rd_regs_i2c.isra.1' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/rtl2830.c:56:1: warning: 'rtl2830_wr' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/rtl2832.c:187:1: warning: 'rtl2832_wr' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/tda10071.c:52:1: warning: 'tda10071_wr_regs' uses dynamic stack allocation [enabled by default]
> drivers/media/dvb-frontends/tda10071.c:84:1: warning: 'tda10071_rd_regs' 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 <m.chehab@samsung.com>
> ---
> drivers/media/dvb-frontends/af9013.c | 12 +++++++++++-
> drivers/media/dvb-frontends/af9033.c | 21 +++++++++++++++++++--
> drivers/media/dvb-frontends/cxd2820r_core.c | 21 +++++++++++++++++++--
> drivers/media/dvb-frontends/rtl2830.c | 12 +++++++++++-
> drivers/media/dvb-frontends/rtl2832.c | 12 +++++++++++-
> drivers/media/dvb-frontends/tda10071.c | 21 +++++++++++++++++++--
> 6 files changed, 90 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
> index a204f2828820..19ba66ad23fa 100644
> --- a/drivers/media/dvb-frontends/af9013.c
> +++ b/drivers/media/dvb-frontends/af9013.c
> @@ -24,6 +24,9 @@
>
> #include "af9013_priv.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> struct af9013_state {
> struct i2c_adapter *i2c;
> struct dvb_frontend fe;
> @@ -50,7 +53,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
> const u8 *val, int len)
> {
> int ret;
> - u8 buf[3+len];
> + u8 buf[MAX_XFER_SIZE];
> struct i2c_msg msg[1] = {
> {
> .addr = priv->config.i2c_addr,
> @@ -60,6 +63,13 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
> }
> };
>
> + if (3 + 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 >> 8) & 0xff;
> buf[1] = (reg >> 0) & 0xff;
> buf[2] = mbox;
> diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c
> index a777b4b944eb..11f1555e66dc 100644
> --- a/drivers/media/dvb-frontends/af9033.c
> +++ b/drivers/media/dvb-frontends/af9033.c
> @@ -21,6 +21,9 @@
>
> #include "af9033_priv.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> struct af9033_state {
> struct i2c_adapter *i2c;
> struct dvb_frontend fe;
> @@ -40,7 +43,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
> int len)
> {
> int ret;
> - u8 buf[3 + len];
> + u8 buf[MAX_XFER_SIZE];
> struct i2c_msg msg[1] = {
> {
> .addr = state->cfg.i2c_addr,
> @@ -50,6 +53,13 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
> }
> };
>
> + if (3 + len > sizeof(buf)) {
> + dev_warn(&state->i2c->dev,
> + "%s: i2c wr reg=%04x: len=%d is too big!\n",
> + KBUILD_MODNAME, reg, len);
> + return -EINVAL;
> + }
> +
> buf[0] = (reg >> 16) & 0xff;
> buf[1] = (reg >> 8) & 0xff;
> buf[2] = (reg >> 0) & 0xff;
> @@ -161,7 +171,14 @@ static int af9033_wr_reg_val_tab(struct af9033_state *state,
> const struct reg_val *tab, int tab_len)
> {
> int ret, i, j;
> - u8 buf[tab_len];
> + u8 buf[MAX_XFER_SIZE];
> +
> + if (tab_len > sizeof(buf)) {
> + dev_warn(&state->i2c->dev,
> + "%s: i2c wr len=%d is too big!\n",
> + KBUILD_MODNAME, tab_len);
> + return -EINVAL;
> + }
>
> dev_dbg(&state->i2c->dev, "%s: tab_len=%d\n", __func__, tab_len);
>
> diff --git a/drivers/media/dvb-frontends/cxd2820r_core.c b/drivers/media/dvb-frontends/cxd2820r_core.c
> index d9eeeb1dfa96..03930d5e9fea 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_core.c
> +++ b/drivers/media/dvb-frontends/cxd2820r_core.c
> @@ -21,12 +21,15 @@
>
> #include "cxd2820r_priv.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> /* write multiple registers */
> static int cxd2820r_wr_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
> u8 *val, int len)
> {
> int ret;
> - u8 buf[len+1];
> + u8 buf[MAX_XFER_SIZE];
> struct i2c_msg msg[1] = {
> {
> .addr = i2c,
> @@ -36,6 +39,13 @@ static int cxd2820r_wr_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
> }
> };
>
> + 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);
>
> @@ -55,7 +65,7 @@ static int cxd2820r_rd_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
> u8 *val, int len)
> {
> int ret;
> - u8 buf[len];
> + u8 buf[MAX_XFER_SIZE];
> struct i2c_msg msg[2] = {
> {
> .addr = i2c,
> @@ -70,6 +80,13 @@ static int cxd2820r_rd_regs_i2c(struct cxd2820r_priv *priv, u8 i2c, u8 reg,
> }
> };
>
> + if (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, len);
> diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
> index 362d26d11e82..b908800b390d 100644
> --- a/drivers/media/dvb-frontends/rtl2830.c
> +++ b/drivers/media/dvb-frontends/rtl2830.c
> @@ -27,11 +27,14 @@
>
> #include "rtl2830_priv.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> /* write multiple hardware registers */
> static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const 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,
> @@ -41,6 +44,13 @@ static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const u8 *val, int 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;
> + }
> +
> buf[0] = reg;
> memcpy(&buf[1], val, len);
>
> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> index a95dfe0a5ce3..cd1e6965ac11 100644
> --- a/drivers/media/dvb-frontends/rtl2832.c
> +++ b/drivers/media/dvb-frontends/rtl2832.c
> @@ -22,6 +22,9 @@
> #include "dvb_math.h"
> #include <linux/bitops.h>
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> int rtl2832_debug;
> module_param_named(debug, rtl2832_debug, int, 0644);
> MODULE_PARM_DESC(debug, "Turn on/off frontend debugging (default:off).");
> @@ -162,7 +165,7 @@ static const struct rtl2832_reg_entry registers[] = {
> static int rtl2832_wr(struct rtl2832_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,
> @@ -172,6 +175,13 @@ static int rtl2832_wr(struct rtl2832_priv *priv, u8 reg, u8 *val, int 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;
> + }
> +
> buf[0] = reg;
> memcpy(&buf[1], val, len);
>
> diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c
> index e79749cfec81..1d8bc2ea4b10 100644
> --- a/drivers/media/dvb-frontends/tda10071.c
> +++ b/drivers/media/dvb-frontends/tda10071.c
> @@ -20,6 +20,9 @@
>
> #include "tda10071_priv.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> static struct dvb_frontend_ops tda10071_ops;
>
> /* write multiple registers */
> @@ -27,7 +30,7 @@ static int tda10071_wr_regs(struct tda10071_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.demod_i2c_addr,
> @@ -37,6 +40,13 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
> }
> };
>
> + 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);
>
> @@ -56,7 +66,7 @@ static int tda10071_rd_regs(struct tda10071_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.demod_i2c_addr,
> @@ -71,6 +81,13 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
> }
> };
>
> + if (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, len);
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 14/29] [media] stb0899_drv: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (12 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 13/29] " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 15/29] [media] stv0367: " Mauro Carvalho Chehab
` (15 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/stb0899_drv.c:540:1: warning: 'stb0899_write_regs' 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 <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/stb0899_drv.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c
index 3dd5714eadba..07cd5ea7a038 100644
--- a/drivers/media/dvb-frontends/stb0899_drv.c
+++ b/drivers/media/dvb-frontends/stb0899_drv.c
@@ -32,6 +32,9 @@
#include "stb0899_priv.h"
#include "stb0899_reg.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static unsigned int verbose = 0;//1;
module_param(verbose, int, 0644);
@@ -499,7 +502,7 @@ err:
int stb0899_write_regs(struct stb0899_state *state, unsigned int reg, u8 *data, u32 count)
{
int ret;
- u8 buf[2 + count];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg i2c_msg = {
.addr = state->config->demod_address,
.flags = 0,
@@ -507,6 +510,13 @@ int stb0899_write_regs(struct stb0899_state *state, unsigned int reg, u8 *data,
.len = 2 + count
};
+ if (2 + count > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, count);
+ return -EINVAL;
+ }
+
buf[0] = reg >> 8;
buf[1] = reg & 0xff;
memcpy(&buf[2], data, count);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 15/29] [media] stv0367: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (13 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 14/29] [media] stb0899_drv: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 16/29] [media] stv090x: " Mauro Carvalho Chehab
` (14 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/stv0367.c:791:1: warning: 'stv0367_writeregs.constprop.4' 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 <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/stv0367.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/stv0367.c b/drivers/media/dvb-frontends/stv0367.c
index 7b6dba3ce55e..458772739423 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -33,6 +33,9 @@
#include "stv0367_regs.h"
#include "stv0367_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static int stvdebug;
module_param_named(debug, stvdebug, int, 0644);
@@ -767,7 +770,7 @@ static struct st_register def0367cab[STV0367CAB_NBREGS] = {
static
int stv0367_writeregs(struct stv0367_state *state, u16 reg, u8 *data, int len)
{
- u8 buf[len + 2];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = state->config->demod_address,
.flags = 0,
@@ -776,6 +779,14 @@ int stv0367_writeregs(struct stv0367_state *state, u16 reg, u8 *data, int len)
};
int ret;
+ if (2 + len > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, len);
+ return -EINVAL;
+ }
+
+
buf[0] = MSB(reg);
buf[1] = LSB(reg);
memcpy(buf + 2, data, len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 16/29] [media] stv090x: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (14 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 15/29] [media] stv0367: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 17/29] [media] av7110_hw: " Mauro Carvalho Chehab
` (13 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/dvb-frontends/stv090x.c:750:1: warning: 'stv090x_write_regs.constprop.6' 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 <m.chehab@samsung.com>
---
drivers/media/dvb-frontends/stv090x.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/stv090x.c b/drivers/media/dvb-frontends/stv090x.c
index 56d470ad5a82..23e872f84742 100644
--- a/drivers/media/dvb-frontends/stv090x.c
+++ b/drivers/media/dvb-frontends/stv090x.c
@@ -35,6 +35,9 @@
#include "stv090x.h"
#include "stv090x_priv.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static unsigned int verbose;
module_param(verbose, int, 0644);
@@ -722,9 +725,16 @@ static int stv090x_write_regs(struct stv090x_state *state, unsigned int reg, u8
{
const struct stv090x_config *config = state->config;
int ret;
- u8 buf[2 + count];
+ u8 buf[MAX_XFER_SIZE];
struct i2c_msg i2c_msg = { .addr = config->address, .flags = 0, .buf = buf, .len = 2 + count };
+ if (2 + count > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, count);
+ return -EINVAL;
+ }
+
buf[0] = reg >> 8;
buf[1] = reg & 0xff;
memcpy(&buf[2], data, count);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 17/29] [media] av7110_hw: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (15 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 16/29] [media] stv090x: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 18/29] [media] tuners: " Mauro Carvalho Chehab
` (12 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/pci/ttpci/av7110_hw.c:510:1: warning: 'av7110_fw_cmd' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer.
In the specific case of this driver, the maximum fw command size
is 6 + 2, as checked using:
$ git grep -A1 av7110_fw_cmd drivers/media/pci/ttpci/
So, use 8 for the buffer size.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/pci/ttpci/av7110_hw.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/media/pci/ttpci/av7110_hw.c b/drivers/media/pci/ttpci/av7110_hw.c
index f1cbfe526989..6299d5dadb82 100644
--- a/drivers/media/pci/ttpci/av7110_hw.c
+++ b/drivers/media/pci/ttpci/av7110_hw.c
@@ -22,7 +22,7 @@
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
* Or, point your browser to http://www.gnu.org/copyleft/gpl.html
*
- * the project's page is at http://www.linuxtv.org/
+ * the project's page is at http://www.linuxtv.org/
*/
/* for debugging ARM communication: */
@@ -40,6 +40,14 @@
#define _NOHANDSHAKE
+/*
+ * Max transfer size done by av7110_fw_cmd()
+ *
+ * The maximum size passed to this function is 6 bytes. The buffer also
+ * uses two additional ones for type and size. So, 8 bytes is enough.
+ */
+#define MAX_XFER_SIZE 8
+
/****************************************************************************
* DEBI functions
****************************************************************************/
@@ -488,11 +496,18 @@ static int av7110_send_fw_cmd(struct av7110 *av7110, u16* buf, int length)
int av7110_fw_cmd(struct av7110 *av7110, int type, int com, int num, ...)
{
va_list args;
- u16 buf[num + 2];
+ u16 buf[MAX_XFER_SIZE];
int i, ret;
// dprintk(4, "%p\n", av7110);
+ if (2 + num > sizeof(buf)) {
+ printk(KERN_WARNING
+ "%s: %s len=%d is too big!\n",
+ KBUILD_MODNAME, __func__, num);
+ return -EINVAL;
+ }
+
buf[0] = ((type << 8) | com);
buf[1] = num;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (16 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 17/29] [media] av7110_hw: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 17:11 ` Antti Palosaari
2013-11-05 10:01 ` [PATCH v3 19/29] [media] tuner-xc2028: " Mauro Carvalho Chehab
` (11 subsequent siblings)
29 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
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 <m.chehab@samsung.com>
---
drivers/media/tuners/e4000.c | 21 +++++++++++++++++++--
drivers/media/tuners/fc2580.c | 21 +++++++++++++++++++--
drivers/media/tuners/tda18212.c | 21 +++++++++++++++++++--
drivers/media/tuners/tda18218.c | 21 +++++++++++++++++++--
4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index ad9309da4a91..30192463c9e1 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -20,11 +20,14 @@
#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,
@@ -34,6 +37,13 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int 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;
+ }
+
buf[0] = reg;
memcpy(&buf[1], val, len);
@@ -53,7 +63,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,
@@ -68,6 +78,13 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len)
}
};
+ 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 81f38aae9c66..430fa5163ec7 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,7 +44,7 @@
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,
@@ -51,6 +54,13 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int 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;
+ }
+
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,
@@ -84,6 +94,13 @@ static int fc2580_rd_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len)
}
};
+ 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 e4a84ee231cf..b3a4adf9ff8f 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,7 +35,7 @@ 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,
@@ -42,6 +45,13 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
}
};
+ 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,
@@ -76,6 +86,13 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
}
};
+ 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 2d31aeb6b088..7e2b32ee5349 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,
@@ -78,6 +88,13 @@ static int tda18218_rd_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len)
}
};
+ 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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
2013-11-05 10:01 ` [PATCH v3 18/29] [media] tuners: " Mauro Carvalho Chehab
@ 2013-11-05 17:11 ` Antti Palosaari
2013-11-07 18:55 ` Antti Palosaari
0 siblings, 1 reply; 39+ messages in thread
From: Antti Palosaari @ 2013-11-05 17:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Acked-by: Antti Palosaari <crope@iki.fi>
Reviewed-by: Antti Palosaari <crope@iki.fi>
Antti
On 05.11.2013 12:01, Mauro Carvalho Chehab wrote:
> 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 <m.chehab@samsung.com>
> ---
> drivers/media/tuners/e4000.c | 21 +++++++++++++++++++--
> drivers/media/tuners/fc2580.c | 21 +++++++++++++++++++--
> drivers/media/tuners/tda18212.c | 21 +++++++++++++++++++--
> drivers/media/tuners/tda18218.c | 21 +++++++++++++++++++--
> 4 files changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> index ad9309da4a91..30192463c9e1 100644
> --- a/drivers/media/tuners/e4000.c
> +++ b/drivers/media/tuners/e4000.c
> @@ -20,11 +20,14 @@
>
> #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,
> @@ -34,6 +37,13 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int 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;
> + }
> +
> buf[0] = reg;
> memcpy(&buf[1], val, len);
>
> @@ -53,7 +63,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,
> @@ -68,6 +78,13 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len)
> }
> };
>
> + 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 81f38aae9c66..430fa5163ec7 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,7 +44,7 @@
> 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,
> @@ -51,6 +54,13 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int 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;
> + }
> +
> 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,
> @@ -84,6 +94,13 @@ static int fc2580_rd_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len)
> }
> };
>
> + 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 e4a84ee231cf..b3a4adf9ff8f 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,7 +35,7 @@ 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,
> @@ -42,6 +45,13 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
> }
> };
>
> + 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,
> @@ -76,6 +86,13 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
> }
> };
>
> + 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 2d31aeb6b088..7e2b32ee5349 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,
> @@ -78,6 +88,13 @@ static int tda18218_rd_regs(struct tda18218_priv *priv, u8 reg, u8 *val, u8 len)
> }
> };
>
> + 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);
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
2013-11-05 17:11 ` Antti Palosaari
@ 2013-11-07 18:55 ` Antti Palosaari
2013-11-07 21:13 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 39+ messages in thread
From: Antti Palosaari @ 2013-11-07 18:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Mauro,
I just notified these are all broken. The reason is here that I2C
adapter sets I2C operation length using sizeof(buf).
Please take a look of all there patches and check existing use of
sizeof(buf).
regards
Antti
On 05.11.2013 19:11, Antti Palosaari wrote:
> Acked-by: Antti Palosaari <crope@iki.fi>
> Reviewed-by: Antti Palosaari <crope@iki.fi>
>
> Antti
>
> On 05.11.2013 12:01, Mauro Carvalho Chehab wrote:
>> 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 <m.chehab@samsung.com>
>> ---
>> drivers/media/tuners/e4000.c | 21 +++++++++++++++++++--
>> drivers/media/tuners/fc2580.c | 21 +++++++++++++++++++--
>> drivers/media/tuners/tda18212.c | 21 +++++++++++++++++++--
>> drivers/media/tuners/tda18218.c | 21 +++++++++++++++++++--
>> 4 files changed, 76 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
>> index ad9309da4a91..30192463c9e1 100644
>> --- a/drivers/media/tuners/e4000.c
>> +++ b/drivers/media/tuners/e4000.c
>> @@ -20,11 +20,14 @@
>>
>> #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,
>> @@ -34,6 +37,13 @@ static int e4000_wr_regs(struct e4000_priv *priv,
>> u8 reg, u8 *val, int 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;
>> + }
>> +
>> buf[0] = reg;
>> memcpy(&buf[1], val, len);
>>
>> @@ -53,7 +63,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,
>> @@ -68,6 +78,13 @@ static int e4000_rd_regs(struct e4000_priv *priv,
>> u8 reg, u8 *val, int len)
>> }
>> };
>>
>> + 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 81f38aae9c66..430fa5163ec7 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,7 +44,7 @@
>> 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,
>> @@ -51,6 +54,13 @@ static int fc2580_wr_regs(struct fc2580_priv *priv,
>> u8 reg, u8 *val, int 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;
>> + }
>> +
>> 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,
>> @@ -84,6 +94,13 @@ static int fc2580_rd_regs(struct fc2580_priv *priv,
>> u8 reg, u8 *val, int len)
>> }
>> };
>>
>> + 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 e4a84ee231cf..b3a4adf9ff8f 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,7 +35,7 @@ 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,
>> @@ -42,6 +45,13 @@ static int tda18212_wr_regs(struct tda18212_priv
>> *priv, u8 reg, u8 *val,
>> }
>> };
>>
>> + 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,
>> @@ -76,6 +86,13 @@ static int tda18212_rd_regs(struct tda18212_priv
>> *priv, u8 reg, u8 *val,
>> }
>> };
>>
>> + 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 2d31aeb6b088..7e2b32ee5349 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,
>> @@ -78,6 +88,13 @@ static int tda18218_rd_regs(struct tda18218_priv
>> *priv, u8 reg, u8 *val, u8 len)
>> }
>> };
>>
>> + 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);
>>
>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
2013-11-07 18:55 ` Antti Palosaari
@ 2013-11-07 21:13 ` Mauro Carvalho Chehab
2013-11-07 23:27 ` Antti Palosaari
0 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-07 21:13 UTC (permalink / raw)
To: Antti Palosaari
Cc: Andy Walls, Linux Media Mailing List, Mauro Carvalho Chehab
Em Thu, 07 Nov 2013 20:55:17 +0200
Antti Palosaari <crope@iki.fi> escreveu:
> Mauro,
> I just notified these are all broken. The reason is here that I2C
> adapter sets I2C operation length using sizeof(buf).
Gah!
> Please take a look of all there patches and check existing use of
> sizeof(buf).
Thanks for review!
Well not all were broken, as, on most drivers weren't using sizeof().
Anyway, I double-checked everything and fixed the drivers.
Instead of just mailbombing a 29 patch series, it seems better to just
paste here the differences from v4, and add a pointer to a git tree
with the full series of patches:
http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/build-fixes-v4
Enclosed is the diff against v3.
PS.: it also addresses the issue pointed by Andy.
Regards,
Mauro
-
diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
index 19ba66ad23fa..fb504f1e9125 100644
--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -58,7 +58,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
{
.addr = priv->config.i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = 3 + len,
.buf = buf,
}
};
diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c
index 11f1555e66dc..30ee59052157 100644
--- a/drivers/media/dvb-frontends/af9033.c
+++ b/drivers/media/dvb-frontends/af9033.c
@@ -48,7 +48,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
{
.addr = state->cfg.i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = 3 + len,
.buf = buf,
}
};
diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
index b908800b390d..7efb796c472c 100644
--- a/drivers/media/dvb-frontends/rtl2830.c
+++ b/drivers/media/dvb-frontends/rtl2830.c
@@ -39,7 +39,7 @@ static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const u8 *val, int len)
{
.addr = priv->cfg.i2c_addr,
.flags = 0,
- .len = 1+len,
+ .len = 1 + len,
.buf = buf,
}
};
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index cd1e6965ac11..ff73da9365e3 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -170,7 +170,7 @@ static int rtl2832_wr(struct rtl2832_priv *priv, u8 reg, u8 *val, int len)
{
.addr = priv->cfg.i2c_addr,
.flags = 0,
- .len = 1+len,
+ .len = 1 + len,
.buf = buf,
}
};
diff --git a/drivers/media/dvb-frontends/s5h1420.c b/drivers/media/dvb-frontends/s5h1420.c
index 97c400a4297f..93eeaf7118fd 100644
--- a/drivers/media/dvb-frontends/s5h1420.c
+++ b/drivers/media/dvb-frontends/s5h1420.c
@@ -854,7 +854,7 @@ static int s5h1420_tuner_i2c_tuner_xfer(struct i2c_adapter *i2c_adap, struct i2c
memcpy(&m[1], msg, sizeof(struct i2c_msg) * num);
- return i2c_transfer(state->i2c, m, 1+num) == 1 + num ? num : -EIO;
+ return i2c_transfer(state->i2c, m, 1 + num) == 1 + num ? num : -EIO;
}
static struct i2c_algorithm s5h1420_tuner_i2c_algo = {
diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c
index 1d8bc2ea4b10..8ad3a57cf640 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -35,7 +35,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
{
.addr = priv->cfg.demod_i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = 1 + len,
.buf = buf,
}
};
@@ -76,7 +76,7 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
}, {
.addr = priv->cfg.demod_i2c_addr,
.flags = I2C_M_RD,
- .len = sizeof(buf),
+ .len = len,
.buf = buf,
}
};
diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
index 87f5bcf29e90..c1f8cc6f14b2 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -327,7 +327,7 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
struct i2c_client *c;
u8 eedata[256];
- c = kzalloc(sizeof(*c), GFP_ATOMIC);
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name));
c->adapter = &cx->i2c_adap[0];
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 30192463c9e1..c9cc1232f2e5 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -32,7 +32,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len)
{
.addr = priv->cfg->i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = 1 + len,
.buf = buf,
}
};
@@ -73,7 +73,7 @@ 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,
}
};
diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c
index 430fa5163ec7..3aecaf465094 100644
--- a/drivers/media/tuners/fc2580.c
+++ b/drivers/media/tuners/fc2580.c
@@ -49,7 +49,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len)
{
.addr = priv->cfg->i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = 1 + len,
.buf = buf,
}
};
@@ -89,7 +89,7 @@ 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,
}
};
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index b3a4adf9ff8f..abe256e1f843 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -40,7 +40,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
{
.addr = priv->cfg->i2c_address,
.flags = 0,
- .len = sizeof(buf),
+ .len = 1 + len,
.buf = buf,
}
};
@@ -81,7 +81,7 @@ 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,
}
};
diff --git a/drivers/media/tuners/tda18218.c b/drivers/media/tuners/tda18218.c
index 7e2b32ee5349..9300e9361e3b 100644
--- a/drivers/media/tuners/tda18218.c
+++ b/drivers/media/tuners/tda18218.c
@@ -83,7 +83,7 @@ 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,
}
};
Cheers,
Mauro
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
2013-11-07 21:13 ` Mauro Carvalho Chehab
@ 2013-11-07 23:27 ` Antti Palosaari
0 siblings, 0 replies; 39+ messages in thread
From: Antti Palosaari @ 2013-11-07 23:27 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Andy Walls, Linux Media Mailing List, Mauro Carvalho Chehab
Reviewed-by: Antti Palosaari <crope@iki.fi>
Antti
On 07.11.2013 23:13, Mauro Carvalho Chehab wrote:
> Em Thu, 07 Nov 2013 20:55:17 +0200
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> Mauro,
>> I just notified these are all broken. The reason is here that I2C
>> adapter sets I2C operation length using sizeof(buf).
>
> Gah!
>
>> Please take a look of all there patches and check existing use of
>> sizeof(buf).
>
> Thanks for review!
>
> Well not all were broken, as, on most drivers weren't using sizeof().
>
> Anyway, I double-checked everything and fixed the drivers.
>
> Instead of just mailbombing a 29 patch series, it seems better to just
> paste here the differences from v4, and add a pointer to a git tree
> with the full series of patches:
>
> http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/build-fixes-v4
>
> Enclosed is the diff against v3.
>
> PS.: it also addresses the issue pointed by Andy.
>
> Regards,
> Mauro
>
> -
>
> diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
> index 19ba66ad23fa..fb504f1e9125 100644
> --- a/drivers/media/dvb-frontends/af9013.c
> +++ b/drivers/media/dvb-frontends/af9013.c
> @@ -58,7 +58,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg,
> {
> .addr = priv->config.i2c_addr,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 3 + len,
> .buf = buf,
> }
> };
> diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c
> index 11f1555e66dc..30ee59052157 100644
> --- a/drivers/media/dvb-frontends/af9033.c
> +++ b/drivers/media/dvb-frontends/af9033.c
> @@ -48,7 +48,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val,
> {
> .addr = state->cfg.i2c_addr,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 3 + len,
> .buf = buf,
> }
> };
> diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
> index b908800b390d..7efb796c472c 100644
> --- a/drivers/media/dvb-frontends/rtl2830.c
> +++ b/drivers/media/dvb-frontends/rtl2830.c
> @@ -39,7 +39,7 @@ static int rtl2830_wr(struct rtl2830_priv *priv, u8 reg, const u8 *val, int len)
> {
> .addr = priv->cfg.i2c_addr,
> .flags = 0,
> - .len = 1+len,
> + .len = 1 + len,
> .buf = buf,
> }
> };
> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> index cd1e6965ac11..ff73da9365e3 100644
> --- a/drivers/media/dvb-frontends/rtl2832.c
> +++ b/drivers/media/dvb-frontends/rtl2832.c
> @@ -170,7 +170,7 @@ static int rtl2832_wr(struct rtl2832_priv *priv, u8 reg, u8 *val, int len)
> {
> .addr = priv->cfg.i2c_addr,
> .flags = 0,
> - .len = 1+len,
> + .len = 1 + len,
> .buf = buf,
> }
> };
> diff --git a/drivers/media/dvb-frontends/s5h1420.c b/drivers/media/dvb-frontends/s5h1420.c
> index 97c400a4297f..93eeaf7118fd 100644
> --- a/drivers/media/dvb-frontends/s5h1420.c
> +++ b/drivers/media/dvb-frontends/s5h1420.c
> @@ -854,7 +854,7 @@ static int s5h1420_tuner_i2c_tuner_xfer(struct i2c_adapter *i2c_adap, struct i2c
>
> memcpy(&m[1], msg, sizeof(struct i2c_msg) * num);
>
> - return i2c_transfer(state->i2c, m, 1+num) == 1 + num ? num : -EIO;
> + return i2c_transfer(state->i2c, m, 1 + num) == 1 + num ? num : -EIO;
> }
>
> static struct i2c_algorithm s5h1420_tuner_i2c_algo = {
> diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c
> index 1d8bc2ea4b10..8ad3a57cf640 100644
> --- a/drivers/media/dvb-frontends/tda10071.c
> +++ b/drivers/media/dvb-frontends/tda10071.c
> @@ -35,7 +35,7 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
> {
> .addr = priv->cfg.demod_i2c_addr,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 1 + len,
> .buf = buf,
> }
> };
> @@ -76,7 +76,7 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8 reg, u8 *val,
> }, {
> .addr = priv->cfg.demod_i2c_addr,
> .flags = I2C_M_RD,
> - .len = sizeof(buf),
> + .len = len,
> .buf = buf,
> }
> };
> diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
> index 87f5bcf29e90..c1f8cc6f14b2 100644
> --- a/drivers/media/pci/cx18/cx18-driver.c
> +++ b/drivers/media/pci/cx18/cx18-driver.c
> @@ -327,7 +327,7 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv)
> struct i2c_client *c;
> u8 eedata[256];
>
> - c = kzalloc(sizeof(*c), GFP_ATOMIC);
> + c = kzalloc(sizeof(*c), GFP_KERNEL);
>
> strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name));
> c->adapter = &cx->i2c_adap[0];
> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> index 30192463c9e1..c9cc1232f2e5 100644
> --- a/drivers/media/tuners/e4000.c
> +++ b/drivers/media/tuners/e4000.c
> @@ -32,7 +32,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len)
> {
> .addr = priv->cfg->i2c_addr,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 1 + len,
> .buf = buf,
> }
> };
> @@ -73,7 +73,7 @@ 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,
> }
> };
> diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c
> index 430fa5163ec7..3aecaf465094 100644
> --- a/drivers/media/tuners/fc2580.c
> +++ b/drivers/media/tuners/fc2580.c
> @@ -49,7 +49,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len)
> {
> .addr = priv->cfg->i2c_addr,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 1 + len,
> .buf = buf,
> }
> };
> @@ -89,7 +89,7 @@ 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,
> }
> };
> diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
> index b3a4adf9ff8f..abe256e1f843 100644
> --- a/drivers/media/tuners/tda18212.c
> +++ b/drivers/media/tuners/tda18212.c
> @@ -40,7 +40,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
> {
> .addr = priv->cfg->i2c_address,
> .flags = 0,
> - .len = sizeof(buf),
> + .len = 1 + len,
> .buf = buf,
> }
> };
> @@ -81,7 +81,7 @@ 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,
> }
> };
> diff --git a/drivers/media/tuners/tda18218.c b/drivers/media/tuners/tda18218.c
> index 7e2b32ee5349..9300e9361e3b 100644
> --- a/drivers/media/tuners/tda18218.c
> +++ b/drivers/media/tuners/tda18218.c
> @@ -83,7 +83,7 @@ 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,
> }
> };
>
>
> Cheers,
> Mauro
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 19/29] [media] tuner-xc2028: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (17 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 18/29] [media] tuners: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 20/29] [media] cimax2: " Mauro Carvalho Chehab
` (10 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/tuners/tuner-xc2028.c:651:1: warning: 'load_firmware' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer.
In the specific case of this driver, the maximum limit is 80, used only
on tm6000 driver. This limit is due to the size of the USB control URBs.
Ok, it would be theoretically possible to use a bigger size on PCI
devices, but the firmware load time is already good enough. Anyway,
if some usage requires more, it is just a matter of also increasing
the buffer size at load_firmware().
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/tuners/tuner-xc2028.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c
index e287a7417319..4be5cf808a40 100644
--- a/drivers/media/tuners/tuner-xc2028.c
+++ b/drivers/media/tuners/tuner-xc2028.c
@@ -24,6 +24,9 @@
#include <linux/dvb/frontend.h>
#include "dvb_frontend.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 80
+
/* Registers (Write-only) */
#define XREG_INIT 0x00
#define XREG_RF_FREQ 0x02
@@ -547,7 +550,10 @@ static int load_firmware(struct dvb_frontend *fe, unsigned int type,
{
struct xc2028_data *priv = fe->tuner_priv;
int pos, rc;
- unsigned char *p, *endp, buf[priv->ctrl.max_len];
+ unsigned char *p, *endp, buf[MAX_XFER_SIZE];
+
+ if (priv->ctrl.max_len > sizeof(buf))
+ priv->ctrl.max_len = sizeof(buf);
tuner_dbg("%s called\n", __func__);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 20/29] [media] cimax2: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (18 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 19/29] [media] tuner-xc2028: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 21/29] [media] v4l2-async: " Mauro Carvalho Chehab
` (9 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/pci/cx23885/cimax2.c:149:1: warning: 'netup_write_i2c' 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 <m.chehab@samsung.com>
---
drivers/media/pci/cx23885/cimax2.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx23885/cimax2.c b/drivers/media/pci/cx23885/cimax2.c
index 7344849183a7..16fa7ea4d4aa 100644
--- a/drivers/media/pci/cx23885/cimax2.c
+++ b/drivers/media/pci/cx23885/cimax2.c
@@ -26,6 +26,10 @@
#include "cx23885.h"
#include "cimax2.h"
#include "dvb_ca_en50221.h"
+
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
/**** Bit definitions for MC417_RWD and MC417_OEN registers ***
bits 31-16
+-----------+
@@ -125,7 +129,7 @@ static int netup_write_i2c(struct i2c_adapter *i2c_adap, u8 addr, u8 reg,
u8 *buf, int len)
{
int ret;
- u8 buffer[len + 1];
+ u8 buffer[MAX_XFER_SIZE];
struct i2c_msg msg = {
.addr = addr,
@@ -134,6 +138,13 @@ static int netup_write_i2c(struct i2c_adapter *i2c_adap, u8 addr, u8 reg,
.len = len + 1
};
+ if (1 + len > sizeof(buffer)) {
+ printk(KERN_WARNING
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, len);
+ return -EINVAL;
+ }
+
buffer[0] = reg;
memcpy(&buffer[1], buf, len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 21/29] [media] v4l2-async: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (19 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 20/29] [media] cimax2: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 22/29] [media] cxusb: " Mauro Carvalho Chehab
` (8 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/v4l2-core/v4l2-async.c:238:1: warning: 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer.
In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS,
with is currently 128. That means that the buffer size can be up to
128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack.
Worse than that, someone could increase it and cause real troubles.
So, let's use dynamically allocated data, instead.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/v4l2-core/v4l2-async.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index c85d69da35bd..b56c9f300ecb 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -189,23 +189,41 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
struct v4l2_subdev *sd, *tmp;
unsigned int notif_n_subdev = notifier->num_subdevs;
unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
- struct device *dev[n_subdev];
+ struct device **dev;
int i = 0;
if (!notifier->v4l2_dev)
return;
+ dev = kmalloc(n_subdev * sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ dev_err(notifier->v4l2_dev->dev,
+ "Failed to allocate device cache!\n");
+ }
+
mutex_lock(&list_lock);
list_del(¬ifier->list);
list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
- dev[i] = get_device(sd->dev);
+ struct device *d;
+
+ d = get_device(sd->dev);
v4l2_async_cleanup(sd);
/* If we handled USB devices, we'd have to lock the parent too */
- device_release_driver(dev[i++]);
+ device_release_driver(d);
+
+
+ /*
+ * Store device at the device cache, in order to call
+ * put_device() on the final step
+ */
+ if (dev)
+ dev[i++] = d;
+ else
+ put_device(d);
if (notifier->unbind)
notifier->unbind(notifier, sd, sd->asd);
@@ -213,6 +231,12 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
mutex_unlock(&list_lock);
+ /*
+ * Call device_attach() to reprobe devices
+ *
+ * NOTE: If dev allocation fails, i is 0, and the hole loop won't be
+ * executed.
+ */
while (i--) {
struct device *d = dev[i];
@@ -228,6 +252,7 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
}
put_device(d);
}
+ kfree(dev);
notifier->v4l2_dev = NULL;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 22/29] [media] cxusb: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (20 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 21/29] [media] v4l2-async: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 23/29] [media] dibusb-common: " Mauro Carvalho Chehab
` (7 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb/cxusb.c:209:1: warning: 'cxusb_i2c_xfer' uses dynamic stack allocation [enabled by default]
drivers/media/usb/dvb-usb/cxusb.c:69:1: warning: 'cxusb_ctrl_msg' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be the max size of
a control URB payload data (64 bytes).
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb/cxusb.c | 41 +++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c
index 3940bb0f9ef6..20e345d9fe8f 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -43,6 +43,9 @@
#include "lgs8gxx.h"
#include "atbm8830.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
/* debug */
static int dvb_usb_cxusb_debug;
module_param_named(debug, dvb_usb_cxusb_debug, int, 0644);
@@ -57,7 +60,14 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d,
u8 cmd, u8 *wbuf, int wlen, u8 *rbuf, int rlen)
{
int wo = (rbuf == NULL || rlen == 0); /* write-only */
- u8 sndbuf[1+wlen];
+ u8 sndbuf[MAX_XFER_SIZE];
+
+ if (1 + wlen > sizeof(sndbuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ wlen);
+ return -EOPNOTSUPP;
+ }
+
memset(sndbuf, 0, 1+wlen);
sndbuf[0] = cmd;
@@ -158,7 +168,13 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[],
if (msg[i].flags & I2C_M_RD) {
/* read only */
- u8 obuf[3], ibuf[1+msg[i].len];
+ u8 obuf[3], ibuf[MAX_XFER_SIZE];
+
+ if (1 + msg[i].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[i].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = 0;
obuf[1] = msg[i].len;
obuf[2] = msg[i].addr;
@@ -172,7 +188,18 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[],
} else if (i+1 < num && (msg[i+1].flags & I2C_M_RD) &&
msg[i].addr == msg[i+1].addr) {
/* write to then read from same address */
- u8 obuf[3+msg[i].len], ibuf[1+msg[i+1].len];
+ u8 obuf[MAX_XFER_SIZE], ibuf[MAX_XFER_SIZE];
+
+ if (3 + msg[i].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[i].len);
+ return -EOPNOTSUPP;
+ }
+ if (1 + msg[i + 1].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[i + 1].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = msg[i].len;
obuf[1] = msg[i+1].len;
obuf[2] = msg[i].addr;
@@ -191,7 +218,13 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[],
i++;
} else {
/* write only */
- u8 obuf[2+msg[i].len], ibuf;
+ u8 obuf[MAX_XFER_SIZE], ibuf;
+
+ if (2 + msg[i].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[i].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = msg[i].addr;
obuf[1] = msg[i].len;
memcpy(&obuf[2], msg[i].buf, msg[i].len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 23/29] [media] dibusb-common: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (21 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 22/29] [media] cxusb: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 24/29] [media] dw2102: " Mauro Carvalho Chehab
` (6 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb/dibusb-common.c:124:1: warning: 'dibusb_i2c_msg' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be the max size of
a control URB payload data (64 bytes).
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb/dibusb-common.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c
index c2dded92f1d3..6d68af0c49c8 100644
--- a/drivers/media/usb/dvb-usb/dibusb-common.c
+++ b/drivers/media/usb/dvb-usb/dibusb-common.c
@@ -12,6 +12,9 @@
#include <linux/kconfig.h>
#include "dibusb.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "set debugging level (1=info (|-able))." DVB_USB_DEBUG_STATUS);
@@ -105,11 +108,16 @@ EXPORT_SYMBOL(dibusb2_0_power_ctrl);
static int dibusb_i2c_msg(struct dvb_usb_device *d, u8 addr,
u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen)
{
- u8 sndbuf[wlen+4]; /* lead(1) devaddr,direction(1) addr(2) data(wlen) (len(2) (when reading)) */
+ u8 sndbuf[MAX_XFER_SIZE]; /* lead(1) devaddr,direction(1) addr(2) data(wlen) (len(2) (when reading)) */
/* write only ? */
int wo = (rbuf == NULL || rlen == 0),
len = 2 + wlen + (wo ? 0 : 2);
+ if (4 + wlen > sizeof(sndbuf)) {
+ warn("i2c wr: len=%d is too big!\n", wlen);
+ return -EOPNOTSUPP;
+ }
+
sndbuf[0] = wo ? DIBUSB_REQ_I2C_WRITE : DIBUSB_REQ_I2C_READ;
sndbuf[1] = (addr << 1) | (wo ? 0 : 1);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 24/29] [media] dw2102: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (22 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 23/29] [media] dibusb-common: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 25/29] [media] af9015: " Mauro Carvalho Chehab
` (5 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb/dw2102.c:368:1: warning: 'dw2102_earda_i2c_transfer' uses dynamic stack allocation [enabled by default]
drivers/media/usb/dvb-usb/dw2102.c:449:1: warning: 'dw2104_i2c_transfer' uses dynamic stack allocation [enabled by default]
drivers/media/usb/dvb-usb/dw2102.c:512:1: warning: 'dw3101_i2c_transfer' uses dynamic stack allocation [enabled by default]
drivers/media/usb/dvb-usb/dw2102.c:621:1: warning: 's6x0_i2c_transfer' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be the max size of
a control URB payload data (64 bytes).
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb/dw2102.c | 90 +++++++++++++++++++++++++++++++++-----
1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c
index 6136a2c7dbfd..c1a63b2a6baa 100644
--- a/drivers/media/usb/dvb-usb/dw2102.c
+++ b/drivers/media/usb/dvb-usb/dw2102.c
@@ -30,6 +30,9 @@
#include "stb6100_proc.h"
#include "m88rs2000.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
#ifndef USB_PID_DW2102
#define USB_PID_DW2102 0x2102
#endif
@@ -308,7 +311,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms
case 2: {
/* read */
/* first write first register number */
- u8 ibuf[msg[1].len + 2], obuf[3];
+ u8 ibuf[MAX_XFER_SIZE], obuf[3];
+
+ if (2 + msg[1].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[1].len);
+ return -EOPNOTSUPP;
+ }
+
obuf[0] = msg[0].addr << 1;
obuf[1] = msg[0].len;
obuf[2] = msg[0].buf[0];
@@ -325,7 +335,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms
switch (msg[0].addr) {
case 0x68: {
/* write to register */
- u8 obuf[msg[0].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[0].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[1].len);
+ return -EOPNOTSUPP;
+ }
+
obuf[0] = msg[0].addr << 1;
obuf[1] = msg[0].len;
memcpy(obuf + 2, msg[0].buf, msg[0].len);
@@ -335,7 +352,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms
}
case 0x61: {
/* write to tuner */
- u8 obuf[msg[0].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[0].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[1].len);
+ return -EOPNOTSUPP;
+ }
+
obuf[0] = msg[0].addr << 1;
obuf[1] = msg[0].len;
memcpy(obuf + 2, msg[0].buf, msg[0].len);
@@ -401,7 +425,14 @@ static int dw2104_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[], i
default: {
if (msg[j].flags == I2C_M_RD) {
/* read registers */
- u8 ibuf[msg[j].len + 2];
+ u8 ibuf[MAX_XFER_SIZE];
+
+ if (2 + msg[j].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[j].len);
+ return -EOPNOTSUPP;
+ }
+
dw210x_op_rw(d->udev, 0xc3,
(msg[j].addr << 1) + 1, 0,
ibuf, msg[j].len + 2,
@@ -430,7 +461,14 @@ static int dw2104_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[], i
} while (len > 0);
} else {
/* write registers */
- u8 obuf[msg[j].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[j].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[j].len);
+ return -EOPNOTSUPP;
+ }
+
obuf[0] = msg[j].addr << 1;
obuf[1] = msg[j].len;
memcpy(obuf + 2, msg[j].buf, msg[j].len);
@@ -463,7 +501,13 @@ static int dw3101_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[],
case 2: {
/* read */
/* first write first register number */
- u8 ibuf[msg[1].len + 2], obuf[3];
+ u8 ibuf[MAX_XFER_SIZE], obuf[3];
+
+ if (2 + msg[1].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[1].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = msg[0].addr << 1;
obuf[1] = msg[0].len;
obuf[2] = msg[0].buf[0];
@@ -481,7 +525,13 @@ static int dw3101_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[],
case 0x60:
case 0x0c: {
/* write to register */
- u8 obuf[msg[0].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[0].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[0].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = msg[0].addr << 1;
obuf[1] = msg[0].len;
memcpy(obuf + 2, msg[0].buf, msg[0].len);
@@ -563,7 +613,14 @@ static int s6x0_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[],
default: {
if (msg[j].flags == I2C_M_RD) {
/* read registers */
- u8 ibuf[msg[j].len];
+ u8 ibuf[MAX_XFER_SIZE];
+
+ if (msg[j].len > sizeof(ibuf)) {
+ warn("i2c rd: len=%d is too big!\n",
+ msg[j].len);
+ return -EOPNOTSUPP;
+ }
+
dw210x_op_rw(d->udev, 0x91, 0, 0,
ibuf, msg[j].len,
DW210X_READ_MSG);
@@ -590,7 +647,14 @@ static int s6x0_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[],
} while (len > 0);
} else if (j < (num - 1)) {
/* write register addr before read */
- u8 obuf[msg[j].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[j].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[j].len);
+ return -EOPNOTSUPP;
+ }
+
obuf[0] = msg[j + 1].len;
obuf[1] = (msg[j].addr << 1);
memcpy(obuf + 2, msg[j].buf, msg[j].len);
@@ -602,7 +666,13 @@ static int s6x0_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[],
break;
} else {
/* write registers */
- u8 obuf[msg[j].len + 2];
+ u8 obuf[MAX_XFER_SIZE];
+
+ if (2 + msg[j].len > sizeof(obuf)) {
+ warn("i2c wr: len=%d is too big!\n",
+ msg[j].len);
+ return -EOPNOTSUPP;
+ }
obuf[0] = msg[j].len + 1;
obuf[1] = (msg[j].addr << 1);
memcpy(obuf + 2, msg[j].buf, msg[j].len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 25/29] [media] af9015: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (23 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 24/29] [media] dw2102: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 17:14 ` Antti Palosaari
2013-11-05 10:01 ` [PATCH v3 26/29] [media] af9035: " Mauro Carvalho Chehab
` (4 subsequent siblings)
29 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb-v2/af9015.c:433:1: warning: 'af9015_eeprom_hash' uses dynamic stack allocation [enabled by default]
In this specific case, it is a gcc bug, as the size is a const, but
it is easy to just change it from const to a #define, getting rid of
the gcc warning.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb-v2/af9015.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c
index d556042cf312..da47d2392f2a 100644
--- a/drivers/media/usb/dvb-usb-v2/af9015.c
+++ b/drivers/media/usb/dvb-usb-v2/af9015.c
@@ -397,12 +397,13 @@ error:
return ret;
}
+#define AF9015_EEPROM_SIZE 256
+
/* hash (and dump) eeprom */
static int af9015_eeprom_hash(struct dvb_usb_device *d)
{
struct af9015_state *state = d_to_priv(d);
int ret, i;
- static const unsigned int AF9015_EEPROM_SIZE = 256;
u8 buf[AF9015_EEPROM_SIZE];
struct req_t req = {READ_I2C, AF9015_I2C_EEPROM, 0, 0, 1, 1, NULL};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 25/29] [media] af9015: Don't use dynamic static allocation
2013-11-05 10:01 ` [PATCH v3 25/29] [media] af9015: " Mauro Carvalho Chehab
@ 2013-11-05 17:14 ` Antti Palosaari
0 siblings, 0 replies; 39+ messages in thread
From: Antti Palosaari @ 2013-11-05 17:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Acked-by: Antti Palosaari <crope@iki.fi>
Reviewed-by: Antti Palosaari <crope@iki.fi>
Antti
On 05.11.2013 12:01, Mauro Carvalho Chehab wrote:
> Dynamic static allocation is evil, as Kernel stack is too low, and
> compilation complains about it on some archs:
> drivers/media/usb/dvb-usb-v2/af9015.c:433:1: warning: 'af9015_eeprom_hash' uses dynamic stack allocation [enabled by default]
>
> In this specific case, it is a gcc bug, as the size is a const, but
> it is easy to just change it from const to a #define, getting rid of
> the gcc warning.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
> drivers/media/usb/dvb-usb-v2/af9015.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c
> index d556042cf312..da47d2392f2a 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9015.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9015.c
> @@ -397,12 +397,13 @@ error:
> return ret;
> }
>
> +#define AF9015_EEPROM_SIZE 256
> +
> /* hash (and dump) eeprom */
> static int af9015_eeprom_hash(struct dvb_usb_device *d)
> {
> struct af9015_state *state = d_to_priv(d);
> int ret, i;
> - static const unsigned int AF9015_EEPROM_SIZE = 256;
> u8 buf[AF9015_EEPROM_SIZE];
> struct req_t req = {READ_I2C, AF9015_I2C_EEPROM, 0, 0, 1, 1, NULL};
>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 26/29] [media] af9035: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (24 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 25/29] [media] af9015: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 17:17 ` Antti Palosaari
2013-11-05 10:01 ` [PATCH v3 27/29] [media] mxl111sf: " Mauro Carvalho Chehab
` (3 subsequent siblings)
29 siblings, 1 reply; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb-v2/af9035.c:142:1: warning: 'af9035_wr_regs' uses dynamic stack allocation [enabled by default]
drivers/media/usb/dvb-usb-v2/af9035.c:305:1: warning: 'af9035_i2c_master_xfer' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be the max size of
a control URB payload data (64 bytes).
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 1ea17dc2a76e..c8fcd78425bd 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -21,6 +21,9 @@
#include "af9035.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
static u16 af9035_checksum(const u8 *buf, size_t len)
@@ -126,10 +129,16 @@ exit:
/* write multiple registers */
static int af9035_wr_regs(struct dvb_usb_device *d, u32 reg, u8 *val, int len)
{
- u8 wbuf[6 + len];
+ u8 wbuf[MAX_XFER_SIZE];
u8 mbox = (reg >> 16) & 0xff;
struct usb_req req = { CMD_MEM_WR, mbox, sizeof(wbuf), wbuf, 0, NULL };
+ if (6 + len > sizeof(wbuf)) {
+ dev_warn(&d->udev->dev, "%s: i2c wr: len=%d is too big!\n",
+ KBUILD_MODNAME, len);
+ return -EOPNOTSUPP;
+ }
+
wbuf[0] = len;
wbuf[1] = 2;
wbuf[2] = 0;
@@ -228,9 +237,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap,
msg[1].len);
} else {
/* I2C */
- u8 buf[5 + msg[0].len];
+ u8 buf[MAX_XFER_SIZE];
struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
buf, msg[1].len, msg[1].buf };
+
+ if (5 + msg[0].len > sizeof(buf)) {
+ dev_warn(&d->udev->dev,
+ "%s: i2c xfer: len=%d is too big!\n",
+ KBUILD_MODNAME, msg[0].len);
+ return -EOPNOTSUPP;
+ }
req.mbox |= ((msg[0].addr & 0x80) >> 3);
buf[0] = msg[1].len;
buf[1] = msg[0].addr << 1;
@@ -257,9 +273,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap,
msg[0].len - 3);
} else {
/* I2C */
- u8 buf[5 + msg[0].len];
+ u8 buf[MAX_XFER_SIZE];
struct usb_req req = { CMD_I2C_WR, 0, sizeof(buf), buf,
0, NULL };
+
+ if (5 + msg[0].len > sizeof(buf)) {
+ dev_warn(&d->udev->dev,
+ "%s: i2c xfer: len=%d is too big!\n",
+ KBUILD_MODNAME, msg[0].len);
+ return -EOPNOTSUPP;
+ }
req.mbox |= ((msg[0].addr & 0x80) >> 3);
buf[0] = msg[0].len;
buf[1] = msg[0].addr << 1;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 26/29] [media] af9035: Don't use dynamic static allocation
2013-11-05 10:01 ` [PATCH v3 26/29] [media] af9035: " Mauro Carvalho Chehab
@ 2013-11-05 17:17 ` Antti Palosaari
0 siblings, 0 replies; 39+ messages in thread
From: Antti Palosaari @ 2013-11-05 17:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Acked-by: Antti Palosaari <crope@iki.fi>
Reviewed-by: Antti Palosaari <crope@iki.fi>
Antti
On 05.11.2013 12:01, Mauro Carvalho Chehab wrote:
> Dynamic static allocation is evil, as Kernel stack is too low, and
> compilation complains about it on some archs:
> drivers/media/usb/dvb-usb-v2/af9035.c:142:1: warning: 'af9035_wr_regs' uses dynamic stack allocation [enabled by default]
> drivers/media/usb/dvb-usb-v2/af9035.c:305:1: warning: 'af9035_i2c_master_xfer' uses dynamic stack allocation [enabled by default]
>
> Instead, let's enforce a limit for the buffer to be the max size of
> a control URB payload data (64 bytes).
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
> drivers/media/usb/dvb-usb-v2/af9035.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 1ea17dc2a76e..c8fcd78425bd 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -21,6 +21,9 @@
>
> #include "af9035.h"
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE 64
> +
> DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
>
> static u16 af9035_checksum(const u8 *buf, size_t len)
> @@ -126,10 +129,16 @@ exit:
> /* write multiple registers */
> static int af9035_wr_regs(struct dvb_usb_device *d, u32 reg, u8 *val, int len)
> {
> - u8 wbuf[6 + len];
> + u8 wbuf[MAX_XFER_SIZE];
> u8 mbox = (reg >> 16) & 0xff;
> struct usb_req req = { CMD_MEM_WR, mbox, sizeof(wbuf), wbuf, 0, NULL };
>
> + if (6 + len > sizeof(wbuf)) {
> + dev_warn(&d->udev->dev, "%s: i2c wr: len=%d is too big!\n",
> + KBUILD_MODNAME, len);
> + return -EOPNOTSUPP;
> + }
> +
> wbuf[0] = len;
> wbuf[1] = 2;
> wbuf[2] = 0;
> @@ -228,9 +237,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap,
> msg[1].len);
> } else {
> /* I2C */
> - u8 buf[5 + msg[0].len];
> + u8 buf[MAX_XFER_SIZE];
> struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
> buf, msg[1].len, msg[1].buf };
> +
> + if (5 + msg[0].len > sizeof(buf)) {
> + dev_warn(&d->udev->dev,
> + "%s: i2c xfer: len=%d is too big!\n",
> + KBUILD_MODNAME, msg[0].len);
> + return -EOPNOTSUPP;
> + }
> req.mbox |= ((msg[0].addr & 0x80) >> 3);
> buf[0] = msg[1].len;
> buf[1] = msg[0].addr << 1;
> @@ -257,9 +273,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap,
> msg[0].len - 3);
> } else {
> /* I2C */
> - u8 buf[5 + msg[0].len];
> + u8 buf[MAX_XFER_SIZE];
> struct usb_req req = { CMD_I2C_WR, 0, sizeof(buf), buf,
> 0, NULL };
> +
> + if (5 + msg[0].len > sizeof(buf)) {
> + dev_warn(&d->udev->dev,
> + "%s: i2c xfer: len=%d is too big!\n",
> + KBUILD_MODNAME, msg[0].len);
> + return -EOPNOTSUPP;
> + }
> req.mbox |= ((msg[0].addr & 0x80) >> 3);
> buf[0] = msg[0].len;
> buf[1] = msg[0].addr << 1;
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 27/29] [media] mxl111sf: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (25 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 26/29] [media] af9035: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 28/29] [media] lirc_zilog: " Mauro Carvalho Chehab
` (2 subsequent siblings)
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
compilation complains about it on some archs:
drivers/media/usb/dvb-usb-v2/mxl111sf.c:74:1: warning: 'mxl111sf_ctrl_msg' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be the max size of
a control URB payload data (64 bytes).
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/usb/dvb-usb-v2/mxl111sf.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c b/drivers/media/usb/dvb-usb-v2/mxl111sf.c
index e97964ef7f56..2627553f7de1 100644
--- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c
+++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c
@@ -23,6 +23,9 @@
#include "lgdt3305.h"
#include "lg2160.h"
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
int dvb_usb_mxl111sf_debug;
module_param_named(debug, dvb_usb_mxl111sf_debug, int, 0644);
MODULE_PARM_DESC(debug, "set debugging level "
@@ -57,7 +60,12 @@ int mxl111sf_ctrl_msg(struct dvb_usb_device *d,
{
int wo = (rbuf == NULL || rlen == 0); /* write-only */
int ret;
- u8 sndbuf[1+wlen];
+ u8 sndbuf[MAX_XFER_SIZE];
+
+ if (1 + wlen > sizeof(sndbuf)) {
+ pr_warn("%s: len=%d is too big!\n", __func__, wlen);
+ return -EOPNOTSUPP;
+ }
pr_debug("%s(wlen = %d, rlen = %d)\n", __func__, wlen, rlen);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 28/29] [media] lirc_zilog: Don't use dynamic static allocation
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (26 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 27/29] [media] mxl111sf: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 10:01 ` [PATCH v3 29/29] [media] cx18: disable compilation on frv arch Mauro Carvalho Chehab
2013-11-05 15:17 ` [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Hans Verkuil
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
Dynamic static allocation is evil, as Kernel stack is too low, and
ompilation complains about it on some archs:
drivers/staging/media/lirc/lirc_zilog.c:967:1: warning: 'read' uses dynamic stack allocation [enabled by default]
Instead, let's enforce a limit for the buffer to be 64. That should
be more than enough.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/staging/media/lirc/lirc_zilog.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
index 11d5338b4f2f..0feeaadf29dc 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -61,6 +61,9 @@
#include <media/lirc_dev.h>
#include <media/lirc.h>
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE 64
+
struct IR;
struct IR_rx {
@@ -941,7 +944,14 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos)
schedule();
set_current_state(TASK_INTERRUPTIBLE);
} else {
- unsigned char buf[rbuf->chunk_size];
+ unsigned char buf[MAX_XFER_SIZE];
+
+ if (rbuf->chunk_size > sizeof(buf)) {
+ zilog_error("chunk_size is too big (%d)!\n",
+ rbuf->chunk_size);
+ ret = -EINVAL;
+ break;
+ }
m = lirc_buffer_read(rbuf, buf);
if (m == rbuf->chunk_size) {
ret = copy_to_user((void *)outbuf+written, buf,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 29/29] [media] cx18: disable compilation on frv arch
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (27 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 28/29] [media] lirc_zilog: " Mauro Carvalho Chehab
@ 2013-11-05 10:01 ` Mauro Carvalho Chehab
2013-11-05 15:17 ` [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Hans Verkuil
29 siblings, 0 replies; 39+ messages in thread
From: Mauro Carvalho Chehab @ 2013-11-05 10:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab
This driver produces a lot of errors on this arch:
In file included from drivers/media/pci/cx18/cx18-driver.c:26:0:
/drivers/media/pci/cx18/cx18-io.h: In function 'cx18_raw_readl':
drivers/media/pci/cx18/cx18-io.h:40:2: warning: passing argument 1 of '__builtin_read32' discards 'const' qualifier from pointer target type [enabled by default]
arch/frv/include/asm/mb-regs.h:24:15: note: expected 'volatile void *' but argument is of type 'const void *'
...
While this is not fixed, just disable it.
NOTE: I'll likely remove this patch from the final series, as this should
be fixed inside gcc code.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/media/pci/cx18/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/pci/cx18/Kconfig b/drivers/media/pci/cx18/Kconfig
index c675b83c43a9..10e6bc72c460 100644
--- a/drivers/media/pci/cx18/Kconfig
+++ b/drivers/media/pci/cx18/Kconfig
@@ -1,6 +1,7 @@
config VIDEO_CX18
tristate "Conexant cx23418 MPEG encoder support"
depends on VIDEO_V4L2 && DVB_CORE && PCI && I2C
+ depends on !FRV
select I2C_ALGOBIT
select VIDEOBUF_VMALLOC
depends on RC_CORE
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs
2013-11-05 10:01 [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs Mauro Carvalho Chehab
` (28 preceding siblings ...)
2013-11-05 10:01 ` [PATCH v3 29/29] [media] cx18: disable compilation on frv arch Mauro Carvalho Chehab
@ 2013-11-05 15:17 ` Hans Verkuil
29 siblings, 0 replies; 39+ messages in thread
From: Hans Verkuil @ 2013-11-05 15:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Hi Mauro,
For all patches except 22 (which got a newer revision):
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
On 11/05/13 11:01, Mauro Carvalho Chehab wrote:
> To be sure that we're not introducing compilation regressions on media, I'm now
> using ktest to check for errors/warnings.
>
> My current setup is cross-building on several architectures:
> alpha, arm, avr32, cris (64), frv, i386, ia64, m32r, m68k, mips, openrisc, parisc, s390, sh, sparc, sparc64, uml, x86_64
>
> I tried to enable a few other archs:
> blackfin, cris (32), powerpc (32, 64), tile, xtensa
>
> but they fail to compile with allyesconfig due to non-media related issues.
>
> I'm still unsure about how often I'll be doing it, I intend to run it at least
> by the end of the subsystem merge window (by -rc6 or -rc7), and fix the
> issues found there.
>
> V3: contains fixes for the feedbacks received:
> - I2C client driver now returns -EINVAL;
> - I2C adapter drivers now returns -EOPNOSUPP;
> - a macro was added for the buffers;
> - check for failure on allocation at v4l2-async was added;
> - Used iguanair code from Sean, instead of my code;
> - Most buffer sizes changed to 64 bytes, to match max URB
> control size.
>
> Mauro Carvalho Chehab (28):
> [media] tda9887: remove an warning when compiling for alpha
> [media] radio-shark: remove a warning when CONFIG_PM is not defined
> [media] zoran: don't build it on alpha
> [media] cx18: struct i2c_client is too big for stack
> [media] tef6862: fix warning on avr32 arch
> [media] platform drivers: Fix build on frv arch
> [media] radio-si470x-i2c: fix a warning on ia64
> [media] rc: Fir warnings on m68k arch
> [media] uvc/lirc_serial: Fix some warnings on parisc arch
> [media] s5h1420: Don't use dynamic static allocation
> [media] dvb-frontends: Don't use dynamic static allocation
> [media] dvb-frontends: Don't use dynamic static allocation
> [media] stb0899_drv: Don't use dynamic static allocation
> [media] stv0367: Don't use dynamic static allocation
> [media] stv090x: Don't use dynamic static allocation
> [media] av7110_hw: Don't use dynamic static allocation
> [media] tuners: Don't use dynamic static allocation
> [media] tuner-xc2028: Don't use dynamic static allocation
> [media] cimax2: Don't use dynamic static allocation
> [media] v4l2-async: Don't use dynamic static allocation
> [media] cxusb: Don't use dynamic static allocation
> [media] dibusb-common: Don't use dynamic static allocation
> [media] dw2102: Don't use dynamic static allocation
> [media] af9015: Don't use dynamic static allocation
> [media] af9035: Don't use dynamic static allocation
> [media] mxl111sf: Don't use dynamic static allocation
> [media] lirc_zilog: Don't use dynamic static allocation
> [media] cx18: disable compilation on frv arch
>
> Sean Young (1):
> [media] iguanair: simplify calculation of carrier delay cycles
>
> drivers/media/dvb-frontends/af9013.c | 12 +++-
> drivers/media/dvb-frontends/af9033.c | 21 ++++++-
> drivers/media/dvb-frontends/bcm3510.c | 15 ++++-
> drivers/media/dvb-frontends/cxd2820r_core.c | 21 ++++++-
> drivers/media/dvb-frontends/itd1000.c | 13 +++-
> drivers/media/dvb-frontends/mt312.c | 10 ++-
> drivers/media/dvb-frontends/nxt200x.c | 11 +++-
> drivers/media/dvb-frontends/rtl2830.c | 12 +++-
> drivers/media/dvb-frontends/rtl2832.c | 12 +++-
> drivers/media/dvb-frontends/s5h1420.c | 9 ++-
> drivers/media/dvb-frontends/stb0899_drv.c | 12 +++-
> drivers/media/dvb-frontends/stb6100.c | 11 +++-
> drivers/media/dvb-frontends/stv0367.c | 13 +++-
> drivers/media/dvb-frontends/stv090x.c | 12 +++-
> drivers/media/dvb-frontends/stv6110.c | 12 +++-
> drivers/media/dvb-frontends/stv6110x.c | 13 +++-
> drivers/media/dvb-frontends/tda10071.c | 21 ++++++-
> drivers/media/dvb-frontends/tda18271c2dd.c | 14 ++++-
> drivers/media/dvb-frontends/zl10039.c | 12 +++-
> drivers/media/pci/cx18/Kconfig | 1 +
> drivers/media/pci/cx18/cx18-driver.c | 20 +++---
> drivers/media/pci/cx23885/cimax2.c | 13 +++-
> drivers/media/pci/ttpci/av7110_hw.c | 19 +++++-
> drivers/media/pci/zoran/Kconfig | 1 +
> drivers/media/platform/soc_camera/rcar_vin.c | 1 +
> drivers/media/radio/radio-shark.c | 2 +
> drivers/media/radio/radio-shark2.c | 2 +
> drivers/media/radio/si470x/radio-si470x-i2c.c | 4 +-
> drivers/media/radio/tef6862.c | 20 +++---
> drivers/media/rc/fintek-cir.h | 4 +-
> drivers/media/rc/iguanair.c | 22 ++-----
> drivers/media/rc/nuvoton-cir.h | 4 +-
> drivers/media/tuners/e4000.c | 21 ++++++-
> drivers/media/tuners/fc2580.c | 21 ++++++-
> drivers/media/tuners/tda18212.c | 21 ++++++-
> drivers/media/tuners/tda18218.c | 21 ++++++-
> drivers/media/tuners/tda9887.c | 4 +-
> drivers/media/tuners/tuner-xc2028.c | 8 ++-
> drivers/media/usb/dvb-usb-v2/af9015.c | 3 +-
> drivers/media/usb/dvb-usb-v2/af9035.c | 29 ++++++++-
> drivers/media/usb/dvb-usb-v2/mxl111sf.c | 10 ++-
> drivers/media/usb/dvb-usb/cxusb.c | 41 ++++++++++--
> drivers/media/usb/dvb-usb/dibusb-common.c | 10 ++-
> drivers/media/usb/dvb-usb/dw2102.c | 90 ++++++++++++++++++++++++---
> drivers/media/usb/uvc/uvc_video.c | 3 +-
> drivers/media/v4l2-core/v4l2-async.c | 31 ++++++++-
> drivers/staging/media/lirc/lirc_serial.c | 9 ++-
> drivers/staging/media/lirc/lirc_zilog.c | 12 +++-
> 48 files changed, 598 insertions(+), 105 deletions(-)
>
^ permalink raw reply [flat|nested] 39+ messages in thread