[PATCH v2] [media] au8522: Avoid memory leak for device config data
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
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