* [PATCH] checkstack fixes for drivers/media/dvb
@ 2010-05-12 18:55 Prarit Bhargava
2010-05-12 20:39 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 3+ messages in thread
From: Prarit Bhargava @ 2010-05-12 18:55 UTC (permalink / raw)
To: linux-media, mchehab; +Cc: Prarit Bhargava
When compiling 2.6.34-rc7 I see the following warnings
drivers/media/dvb/frontends/dib3000mc.c: In function 'dib3000mc_i2c_enumeration':
drivers/media/dvb/frontends/dib3000mc.c:853: warning: the frame size of 2224 bytes is larger than 2048 bytes
drivers/media/dvb/frontends/dib7000p.c: In function 'dib7000p_i2c_enumeration':
drivers/media/dvb/frontends/dib7000p.c:1346: warning: the frame size of 2304 bytes is larger than 2048 bytes
because the dib*_state structs are large and they are alloc'd on the stack.
This patch moves the structures off the stack.
I also noticed that the cxusb driver doesn't check the return value from
dib7000p_i2c_enumeration().
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
diff --git a/drivers/media/dvb/dvb-usb/cxusb.c b/drivers/media/dvb/dvb-usb/cxusb.c
index 960376d..8141b10 100644
--- a/drivers/media/dvb/dvb-usb/cxusb.c
+++ b/drivers/media/dvb/dvb-usb/cxusb.c
@@ -1025,8 +1025,11 @@ static int cxusb_dualdig4_rev2_frontend_attach(struct dvb_usb_adapter *adap)
cxusb_bluebird_gpio_pulse(adap->dev, 0x02, 1);
- dib7000p_i2c_enumeration(&adap->dev->i2c_adap, 1, 18,
- &cxusb_dualdig4_rev2_config);
+ if (dib7000p_i2c_enumeration(&adap->dev->i2c_adap, 1, 18,
+ &cxusb_dualdig4_rev2_config)) {
+ printk(KERN_WARNING "Unable to enumerate dib7000p\n");
+ return -ENODEV;
+ }
adap->fe = dvb_attach(dib7000p_attach, &adap->dev->i2c_adap, 0x80,
&cxusb_dualdig4_rev2_config);
diff --git a/drivers/media/dvb/frontends/dib3000mc.c b/drivers/media/dvb/frontends/dib3000mc.c
index 40a0998..6a178f1 100644
--- a/drivers/media/dvb/frontends/dib3000mc.c
+++ b/drivers/media/dvb/frontends/dib3000mc.c
@@ -814,42 +814,52 @@ EXPORT_SYMBOL(dib3000mc_set_config);
int dib3000mc_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods, u8 default_addr, struct dib3000mc_config cfg[])
{
- struct dib3000mc_state st = { .i2c_adap = i2c };
+ struct dib3000mc_state *st;
int k;
u8 new_addr;
-
static u8 DIB3000MC_I2C_ADDRESS[] = {20,22,24,26};
+ st = kzalloc(sizeof(*st), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;
+ st->i2c_adap = i2c;
+
for (k = no_of_demods-1; k >= 0; k--) {
- st.cfg = &cfg[k];
+ st->cfg = &cfg[k];
/* designated i2c address */
- new_addr = DIB3000MC_I2C_ADDRESS[k];
- st.i2c_addr = new_addr;
- if (dib3000mc_identify(&st) != 0) {
- st.i2c_addr = default_addr;
- if (dib3000mc_identify(&st) != 0) {
- dprintk("-E- DiB3000P/MC #%d: not identified\n", k);
+ new_addr = DIB3000MC_I2C_ADDRESS[k];
+ st->i2c_addr = new_addr;
+ if (dib3000mc_identify(st) != 0) {
+ st->i2c_addr = default_addr;
+ if (dib3000mc_identify(st) != 0) {
+ dprintk("-E- DiB3000P/MC #%d: not"
+ " identified\n", k);
+ kfree(st);
return -ENODEV;
}
}
- dib3000mc_set_output_mode(&st, OUTMODE_MPEG2_PAR_CONT_CLK);
+ dib3000mc_set_output_mode(st, OUTMODE_MPEG2_PAR_CONT_CLK);
- // set new i2c address and force divstr (Bit 1) to value 0 (Bit 0)
- dib3000mc_write_word(&st, 1024, (new_addr << 3) | 0x1);
- st.i2c_addr = new_addr;
+ /* set new i2c address and force divstr (Bit 1) to
+ * value 0 (Bit 0)
+ */
+ dib3000mc_write_word(st, 1024, (new_addr << 3) | 0x1);
+ st->i2c_addr = new_addr;
}
for (k = 0; k < no_of_demods; k++) {
- st.cfg = &cfg[k];
- st.i2c_addr = DIB3000MC_I2C_ADDRESS[k];
+ st->cfg = &cfg[k];
+ st->i2c_addr = DIB3000MC_I2C_ADDRESS[k];
- dib3000mc_write_word(&st, 1024, st.i2c_addr << 3);
+ dib3000mc_write_word(st, 1024, st->i2c_addr << 3);
/* turn off data output */
- dib3000mc_set_output_mode(&st, OUTMODE_HIGH_Z);
+ dib3000mc_set_output_mode(st, OUTMODE_HIGH_Z);
}
+
+ kfree(st);
return 0;
}
EXPORT_SYMBOL(dib3000mc_i2c_enumeration);
diff --git a/drivers/media/dvb/frontends/dib7000p.c b/drivers/media/dvb/frontends/dib7000p.c
index 85468a4..08ea982 100644
--- a/drivers/media/dvb/frontends/dib7000p.c
+++ b/drivers/media/dvb/frontends/dib7000p.c
@@ -1322,48 +1322,59 @@ int dib7000p_pid_filter(struct dvb_frontend *fe, u8 id, u16 pid, u8 onoff)
}
EXPORT_SYMBOL(dib7000p_pid_filter);
-int dib7000p_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods, u8 default_addr, struct dib7000p_config cfg[])
+int dib7000p_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods,
+ u8 default_addr, struct dib7000p_config cfg[])
{
- struct dib7000p_state st = { .i2c_adap = i2c };
+ struct dib7000p_state *st;
int k = 0;
u8 new_addr = 0;
+ st = kmalloc(sizeof(*st), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;
+ st->i2c_adap = i2c;
+
for (k = no_of_demods-1; k >= 0; k--) {
- st.cfg = cfg[k];
+ st->cfg = cfg[k];
/* designated i2c address */
- new_addr = (0x40 + k) << 1;
- st.i2c_addr = new_addr;
- dib7000p_write_word(&st, 1287, 0x0003); /* sram lead in, rdy */
- if (dib7000p_identify(&st) != 0) {
- st.i2c_addr = default_addr;
- dib7000p_write_word(&st, 1287, 0x0003); /* sram lead in, rdy */
- if (dib7000p_identify(&st) != 0) {
+ new_addr = (0x40 + k) << 1;
+ st->i2c_addr = new_addr;
+ dib7000p_write_word(st, 1287, 0x0003); /* sram lead in, rdy */
+ if (dib7000p_identify(st) != 0) {
+ st->i2c_addr = default_addr;
+ /* sram lead in, rdy */
+ dib7000p_write_word(st, 1287, 0x0003);
+ if (dib7000p_identify(st) != 0) {
dprintk("DiB7000P #%d: not identified\n", k);
+ kfree(st);
return -EIO;
}
}
- /* start diversity to pull_down div_str - just for i2c-enumeration */
- dib7000p_set_output_mode(&st, OUTMODE_DIVERSITY);
+ /* start diversity to pull_down div_str - just for
+ * i2c-enumeration
+ */
+ dib7000p_set_output_mode(st, OUTMODE_DIVERSITY);
/* set new i2c address and force divstart */
- dib7000p_write_word(&st, 1285, (new_addr << 2) | 0x2);
+ dib7000p_write_word(st, 1285, (new_addr << 2) | 0x2);
dprintk("IC %d initialized (to i2c_address 0x%x)", k, new_addr);
}
for (k = 0; k < no_of_demods; k++) {
- st.cfg = cfg[k];
- st.i2c_addr = (0x40 + k) << 1;
+ st->cfg = cfg[k];
+ st->i2c_addr = (0x40 + k) << 1;
- // unforce divstr
- dib7000p_write_word(&st, 1285, st.i2c_addr << 2);
+ /* unforce divstr */
+ dib7000p_write_word(st, 1285, st->i2c_addr << 2);
/* deactivate div - it was just for i2c-enumeration */
- dib7000p_set_output_mode(&st, OUTMODE_HIGH_Z);
+ dib7000p_set_output_mode(st, OUTMODE_HIGH_Z);
}
+ kfree(st);
return 0;
}
EXPORT_SYMBOL(dib7000p_i2c_enumeration);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] checkstack fixes for drivers/media/dvb
2010-05-12 18:55 [PATCH] checkstack fixes for drivers/media/dvb Prarit Bhargava
@ 2010-05-12 20:39 ` Mauro Carvalho Chehab
2010-05-12 22:23 ` Prarit Bhargava
0 siblings, 1 reply; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2010-05-12 20:39 UTC (permalink / raw)
To: Prarit Bhargava; +Cc: linux-media
Prarit Bhargava wrote:
> When compiling 2.6.34-rc7 I see the following warnings
>
> drivers/media/dvb/frontends/dib3000mc.c: In function 'dib3000mc_i2c_enumeration':
> drivers/media/dvb/frontends/dib3000mc.c:853: warning: the frame size of 2224 bytes is larger than 2048 bytes
> drivers/media/dvb/frontends/dib7000p.c: In function 'dib7000p_i2c_enumeration':
> drivers/media/dvb/frontends/dib7000p.c:1346: warning: the frame size of 2304 bytes is larger than 2048 bytes
>
> because the dib*_state structs are large and they are alloc'd on the stack.
>
> This patch moves the structures off the stack.
Hi Prarit,
Thanks for the patch, but I've received two patches to fix the same thing some time ago.
Unfortunately, it took a long time to be merged, since I was waiting for the driver
maintainer's ack. It is at those changesets:
http://git.linuxtv.org/v4l-dvb.git?a=commit;h=65483f7e5f3e169ea038de26068395231dd3b13b
http://git.linuxtv.org/v4l-dvb.git?a=commit;h=370c0cb185d4fccfb2c66fbe94b48579d4c5fa1c
>
> I also noticed that the cxusb driver doesn't check the return value from
> dib7000p_i2c_enumeration().
Randy's patch also added a test for it, but without the warning printk. It may be a good
idea to have that warning. So, please be free to send it as a separate patch if you also
think so.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>
> diff --git a/drivers/media/dvb/dvb-usb/cxusb.c b/drivers/media/dvb/dvb-usb/cxusb.c
> index 960376d..8141b10 100644
> --- a/drivers/media/dvb/dvb-usb/cxusb.c
> +++ b/drivers/media/dvb/dvb-usb/cxusb.c
> @@ -1025,8 +1025,11 @@ static int cxusb_dualdig4_rev2_frontend_attach(struct dvb_usb_adapter *adap)
>
> cxusb_bluebird_gpio_pulse(adap->dev, 0x02, 1);
>
> - dib7000p_i2c_enumeration(&adap->dev->i2c_adap, 1, 18,
> - &cxusb_dualdig4_rev2_config);
> + if (dib7000p_i2c_enumeration(&adap->dev->i2c_adap, 1, 18,
> + &cxusb_dualdig4_rev2_config)) {
> + printk(KERN_WARNING "Unable to enumerate dib7000p\n");
> + return -ENODEV;
> + }
>
> adap->fe = dvb_attach(dib7000p_attach, &adap->dev->i2c_adap, 0x80,
> &cxusb_dualdig4_rev2_config);
> diff --git a/drivers/media/dvb/frontends/dib3000mc.c b/drivers/media/dvb/frontends/dib3000mc.c
> index 40a0998..6a178f1 100644
> --- a/drivers/media/dvb/frontends/dib3000mc.c
> +++ b/drivers/media/dvb/frontends/dib3000mc.c
> @@ -814,42 +814,52 @@ EXPORT_SYMBOL(dib3000mc_set_config);
>
> int dib3000mc_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods, u8 default_addr, struct dib3000mc_config cfg[])
> {
> - struct dib3000mc_state st = { .i2c_adap = i2c };
> + struct dib3000mc_state *st;
> int k;
> u8 new_addr;
> -
> static u8 DIB3000MC_I2C_ADDRESS[] = {20,22,24,26};
>
> + st = kzalloc(sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> + st->i2c_adap = i2c;
> +
> for (k = no_of_demods-1; k >= 0; k--) {
> - st.cfg = &cfg[k];
> + st->cfg = &cfg[k];
>
> /* designated i2c address */
> - new_addr = DIB3000MC_I2C_ADDRESS[k];
> - st.i2c_addr = new_addr;
> - if (dib3000mc_identify(&st) != 0) {
> - st.i2c_addr = default_addr;
> - if (dib3000mc_identify(&st) != 0) {
> - dprintk("-E- DiB3000P/MC #%d: not identified\n", k);
> + new_addr = DIB3000MC_I2C_ADDRESS[k];
> + st->i2c_addr = new_addr;
> + if (dib3000mc_identify(st) != 0) {
> + st->i2c_addr = default_addr;
> + if (dib3000mc_identify(st) != 0) {
> + dprintk("-E- DiB3000P/MC #%d: not"
> + " identified\n", k);
> + kfree(st);
> return -ENODEV;
> }
> }
>
> - dib3000mc_set_output_mode(&st, OUTMODE_MPEG2_PAR_CONT_CLK);
> + dib3000mc_set_output_mode(st, OUTMODE_MPEG2_PAR_CONT_CLK);
>
> - // set new i2c address and force divstr (Bit 1) to value 0 (Bit 0)
> - dib3000mc_write_word(&st, 1024, (new_addr << 3) | 0x1);
> - st.i2c_addr = new_addr;
> + /* set new i2c address and force divstr (Bit 1) to
> + * value 0 (Bit 0)
> + */
> + dib3000mc_write_word(st, 1024, (new_addr << 3) | 0x1);
> + st->i2c_addr = new_addr;
> }
>
> for (k = 0; k < no_of_demods; k++) {
> - st.cfg = &cfg[k];
> - st.i2c_addr = DIB3000MC_I2C_ADDRESS[k];
> + st->cfg = &cfg[k];
> + st->i2c_addr = DIB3000MC_I2C_ADDRESS[k];
>
> - dib3000mc_write_word(&st, 1024, st.i2c_addr << 3);
> + dib3000mc_write_word(st, 1024, st->i2c_addr << 3);
>
> /* turn off data output */
> - dib3000mc_set_output_mode(&st, OUTMODE_HIGH_Z);
> + dib3000mc_set_output_mode(st, OUTMODE_HIGH_Z);
> }
> +
> + kfree(st);
> return 0;
> }
> EXPORT_SYMBOL(dib3000mc_i2c_enumeration);
> diff --git a/drivers/media/dvb/frontends/dib7000p.c b/drivers/media/dvb/frontends/dib7000p.c
> index 85468a4..08ea982 100644
> --- a/drivers/media/dvb/frontends/dib7000p.c
> +++ b/drivers/media/dvb/frontends/dib7000p.c
> @@ -1322,48 +1322,59 @@ int dib7000p_pid_filter(struct dvb_frontend *fe, u8 id, u16 pid, u8 onoff)
> }
> EXPORT_SYMBOL(dib7000p_pid_filter);
>
> -int dib7000p_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods, u8 default_addr, struct dib7000p_config cfg[])
> +int dib7000p_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods,
> + u8 default_addr, struct dib7000p_config cfg[])
> {
> - struct dib7000p_state st = { .i2c_adap = i2c };
> + struct dib7000p_state *st;
> int k = 0;
> u8 new_addr = 0;
>
> + st = kmalloc(sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> + st->i2c_adap = i2c;
> +
> for (k = no_of_demods-1; k >= 0; k--) {
> - st.cfg = cfg[k];
> + st->cfg = cfg[k];
>
> /* designated i2c address */
> - new_addr = (0x40 + k) << 1;
> - st.i2c_addr = new_addr;
> - dib7000p_write_word(&st, 1287, 0x0003); /* sram lead in, rdy */
> - if (dib7000p_identify(&st) != 0) {
> - st.i2c_addr = default_addr;
> - dib7000p_write_word(&st, 1287, 0x0003); /* sram lead in, rdy */
> - if (dib7000p_identify(&st) != 0) {
> + new_addr = (0x40 + k) << 1;
> + st->i2c_addr = new_addr;
> + dib7000p_write_word(st, 1287, 0x0003); /* sram lead in, rdy */
> + if (dib7000p_identify(st) != 0) {
> + st->i2c_addr = default_addr;
> + /* sram lead in, rdy */
> + dib7000p_write_word(st, 1287, 0x0003);
> + if (dib7000p_identify(st) != 0) {
> dprintk("DiB7000P #%d: not identified\n", k);
> + kfree(st);
> return -EIO;
> }
> }
>
> - /* start diversity to pull_down div_str - just for i2c-enumeration */
> - dib7000p_set_output_mode(&st, OUTMODE_DIVERSITY);
> + /* start diversity to pull_down div_str - just for
> + * i2c-enumeration
> + */
> + dib7000p_set_output_mode(st, OUTMODE_DIVERSITY);
>
> /* set new i2c address and force divstart */
> - dib7000p_write_word(&st, 1285, (new_addr << 2) | 0x2);
> + dib7000p_write_word(st, 1285, (new_addr << 2) | 0x2);
>
> dprintk("IC %d initialized (to i2c_address 0x%x)", k, new_addr);
> }
>
> for (k = 0; k < no_of_demods; k++) {
> - st.cfg = cfg[k];
> - st.i2c_addr = (0x40 + k) << 1;
> + st->cfg = cfg[k];
> + st->i2c_addr = (0x40 + k) << 1;
>
> - // unforce divstr
> - dib7000p_write_word(&st, 1285, st.i2c_addr << 2);
> + /* unforce divstr */
> + dib7000p_write_word(st, 1285, st->i2c_addr << 2);
>
> /* deactivate div - it was just for i2c-enumeration */
> - dib7000p_set_output_mode(&st, OUTMODE_HIGH_Z);
> + dib7000p_set_output_mode(st, OUTMODE_HIGH_Z);
> }
>
> + kfree(st);
> return 0;
> }
> EXPORT_SYMBOL(dib7000p_i2c_enumeration);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Cheers,
Mauro
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] checkstack fixes for drivers/media/dvb
2010-05-12 20:39 ` Mauro Carvalho Chehab
@ 2010-05-12 22:23 ` Prarit Bhargava
0 siblings, 0 replies; 3+ messages in thread
From: Prarit Bhargava @ 2010-05-12 22:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media
On 05/12/2010 04:39 PM, Mauro Carvalho Chehab wrote:
> Prarit Bhargava wrote:
>
>> When compiling 2.6.34-rc7 I see the following warnings
>>
>> drivers/media/dvb/frontends/dib3000mc.c: In function 'dib3000mc_i2c_enumeration':
>> drivers/media/dvb/frontends/dib3000mc.c:853: warning: the frame size of 2224 bytes is larger than 2048 bytes
>> drivers/media/dvb/frontends/dib7000p.c: In function 'dib7000p_i2c_enumeration':
>> drivers/media/dvb/frontends/dib7000p.c:1346: warning: the frame size of 2304 bytes is larger than 2048 bytes
>>
>> because the dib*_state structs are large and they are alloc'd on the stack.
>>
>> This patch moves the structures off the stack.
>>
> Hi Prarit,
>
> Thanks for the patch, but I've received two patches to fix the same thing some time ago.
> Unfortunately, it took a long time to be merged, since I was waiting for the driver
> maintainer's ack. It is at those changesets:
>
> http://git.linuxtv.org/v4l-dvb.git?a=commit;h=65483f7e5f3e169ea038de26068395231dd3b13b
> http://git.linuxtv.org/v4l-dvb.git?a=commit;h=370c0cb185d4fccfb2c66fbe94b48579d4c5fa1c
>
>
Oops! Sorry about that Mauro :( -- I didn't realize there was another
git tree to check. Just curious -- is the one listed in MAINTAINERS
still active?
>> I also noticed that the cxusb driver doesn't check the return value from
>> dib7000p_i2c_enumeration().
>>
> Randy's patch also added a test for it, but without the warning printk. It may be a good
> idea to have that warning. So, please be free to send it as a separate patch if you also
> think so.
>
>
Sure I'll do that shortly (after checking out the above tree ;) ).
P.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-05-12 22:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-12 18:55 [PATCH] checkstack fixes for drivers/media/dvb Prarit Bhargava
2010-05-12 20:39 ` Mauro Carvalho Chehab
2010-05-12 22:23 ` Prarit Bhargava
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).