[PATCH v2] [media] au8522: Avoid memory leak for device config data

2015-12-21 Thread Mauro Carvalho Chehab
As reported by kmemleak:

unreferenced object 0x880321e1da40 (size 32):
  comm "modprobe", pid 3309, jiffies 4295019569 (age 2359.636s)
  hex dump (first 32 bytes):
47 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  G...
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
  backtrace:
[] kmemleak_alloc+0x4e/0xb0
[] kmem_cache_alloc_trace+0x1ec/0x280
[] au8522_probe+0x19a/0xa30 [au8522_decoder]
[] i2c_device_probe+0x2b2/0x490
[] driver_probe_device+0x454/0xd90
[] __device_attach_driver+0x17b/0x230
[] bus_for_each_drv+0x11a/0x1b0
[] __device_attach+0x1cd/0x2c0
[] device_initial_probe+0x13/0x20
[] bus_probe_device+0x1af/0x250
[] device_add+0x943/0x13b0
[] device_register+0x1a/0x20
[] i2c_new_device+0x5d6/0x8f0
[] v4l2_i2c_new_subdev_board+0x1e4/0x250 
[v4l2_common]
[] v4l2_i2c_new_subdev+0xd7/0x110 [v4l2_common]
[] au0828_card_analog_fe_setup+0x2e6/0x3f0 
[au0828]

Checking where the error happens:
(gdb) list *au8522_probe+0x19a
0x99a is in au8522_probe 
(drivers/media/dvb-frontends/au8522_decoder.c:761).
756 printk(KERN_INFO "au8522_decoder attach 
existing instance.\n");
757 break;
758 }
759
760 demod_config = kzalloc(sizeof(struct au8522_config), 
GFP_KERNEL);
761 if (demod_config == NULL) {
762 if (instance == 1)
763 kfree(state);
764 return -ENOMEM;
765 }

Shows that the error path is not being handled properly.

The are actually several issues here:

1) config free should have been calling hybrid_tuner_release_state()
function, by calling au8522_release_state();

2) config is only allocated at the digital part. On the analog one,
it is received from the caller.

A complex logic could be added to address it, however, it is simpler
to just embeed config inside the state.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/au8522_common.c  | 10 +-
 drivers/media/dvb-frontends/au8522_decoder.c | 10 +-
 drivers/media/dvb-frontends/au8522_dig.c | 16 
 drivers/media/dvb-frontends/au8522_priv.h|  2 +-
 4 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/media/dvb-frontends/au8522_common.c 
b/drivers/media/dvb-frontends/au8522_common.c
index 3559ff230045..f135126bc373 100644
--- a/drivers/media/dvb-frontends/au8522_common.c
+++ b/drivers/media/dvb-frontends/au8522_common.c
@@ -44,7 +44,7 @@ int au8522_writereg(struct au8522_state *state, u16 reg, u8 
data)
int ret;
u8 buf[] = { (reg >> 8) | 0x80, reg & 0xff, data };
 
-   struct i2c_msg msg = { .addr = state->config->demod_address,
+   struct i2c_msg msg = { .addr = state->config.demod_address,
   .flags = 0, .buf = buf, .len = 3 };
 
ret = i2c_transfer(state->i2c, , 1);
@@ -64,9 +64,9 @@ u8 au8522_readreg(struct au8522_state *state, u16 reg)
u8 b1[] = { 0 };
 
struct i2c_msg msg[] = {
-   { .addr = state->config->demod_address, .flags = 0,
+   { .addr = state->config.demod_address, .flags = 0,
  .buf = b0, .len = 2 },
-   { .addr = state->config->demod_address, .flags = I2C_M_RD,
+   { .addr = state->config.demod_address, .flags = I2C_M_RD,
  .buf = b1, .len = 1 } };
 
ret = i2c_transfer(state->i2c, msg, 2);
@@ -140,7 +140,7 @@ EXPORT_SYMBOL(au8522_release_state);
 
 static int au8522_led_gpio_enable(struct au8522_state *state, int onoff)
 {
-   struct au8522_led_config *led_config = state->config->led_cfg;
+   struct au8522_led_config *led_config = state->config.led_cfg;
u8 val;
 
/* bail out if we can't control an LED */
@@ -170,7 +170,7 @@ static int au8522_led_gpio_enable(struct au8522_state 
*state, int onoff)
  */
 int au8522_led_ctrl(struct au8522_state *state, int led)
 {
-   struct au8522_led_config *led_config = state->config->led_cfg;
+   struct au8522_led_config *led_config = state->config.led_cfg;
int i, ret = 0;
 
/* bail out if we can't control an LED */
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c 
b/drivers/media/dvb-frontends/au8522_decoder.c
index 28d7dc2fee34..b3502a6191ba 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -754,15 +754,7 @@ static int au8522_probe(struct i2c_client *client,
break;
}
 
-   demod_config = kzalloc(sizeof(struct au8522_config), GFP_KERNEL);
-   if (demod_config == NULL) {
- 

Re: [PATCH v2] [media] au8522: Avoid memory leak for device config data

2015-12-21 Thread kbuild test robot
Hi Mauro,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.4-rc6 next-20151221]

url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/au8522-Avoid-memory-leak-for-device-config-data/20151222-010649
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x000-201551 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/media/dvb-frontends/au8522_decoder.c: In function 'au8522_probe':
>> drivers/media/dvb-frontends/au8522_decoder.c:779:3: warning: 'demod_config' 
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
  kfree(demod_config);
  ^

vim +/demod_config +779 drivers/media/dvb-frontends/au8522_decoder.c

5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  763 hdl = >hdl;
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  764 v4l2_ctrl_handler_init(hdl, 4);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  765 v4l2_ctrl_new_std(hdl, _ctrl_ops,
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  766 V4L2_CID_BRIGHTNESS, 0, 255, 1, 109);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  767 v4l2_ctrl_new_std(hdl, _ctrl_ops,
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  768 V4L2_CID_CONTRAST, 0, 255, 1,
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  769 AU8522_TVDEC_CONTRAST_REG00BH_CVBS);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  770 v4l2_ctrl_new_std(hdl, _ctrl_ops,
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  771 V4L2_CID_SATURATION, 0, 255, 1, 128);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  772 v4l2_ctrl_new_std(hdl, _ctrl_ops,
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  773 V4L2_CID_HUE, -32768, 32767, 1, 0);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  774 sd->ctrl_handler = hdl;
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  775 if (hdl->error) {
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  776 int err = hdl->error;
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  777  
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  778 v4l2_ctrl_handler_free(hdl);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15 @779 kfree(demod_config);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  780 kfree(state);
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  781 return err;
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  782 }
5a4bdb4b drivers/media/dvb-frontends/au8522_decoder.c Hans Verkuil  
2013-02-15  783  
968cf782 drivers/media/dvb/frontends/au8522_decoder.c Devin Heitmueller 
2009-03-11  784 state->c = client;
f2fd7ce6 drivers/media/dvb-frontends/au8522_decoder.c Mauro Carvalho Chehab 
2014-06-08  785 state->std = V4L2_STD_NTSC_M;
968cf782 drivers/media/dvb/frontends/au8522_decoder.c Devin Heitmueller 
2009-03-11  786 state->vid_input = AU8522_COMPOSITE_CH1;
968cf782 drivers/media/dvb/frontends/au8522_decoder.c Devin Heitmueller 
2009-03-11  787 state->aud_input = AU8522_AUDIO_NONE;

:: The code at line 779 was first introduced by commit
:: 5a4bdb4b34b90655891f627679bbba0ed9791c2e [media] au8522_decoder: convert 
to the control framework

:: TO: Hans Verkuil 
:: CC: Mauro Carvalho Chehab 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data