Prev: [PATCH 7/9] Staging: comedi: cb_das16_cs: fixed multiple brace coding style issue
Next: usb/otg/isp1301_omap: Fix dangling pointer
From: Dmitry Torokhov on 20 Mar 2010 15:30 Hi Wolfram, On Sat, Mar 20, 2010 at 03:12:47PM +0100, Wolfram Sang wrote: > Fix I2C-drivers which missed setting clientdata to NULL before freeing the > structure it points to. Also fix drivers which do this _after_ the structure > was freed already. > I am not sure if setting clientdata to NULL before freeing the data is that important; we really want to be sure that we don't leave clientdata dangling when we finish unbinding the driver. If there are another thread the change will not really help the problem of accessing non-existing client data. I will apply lm8323 portion of the patch but leave qt2160 as is. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Wolfram Sang on 20 Mar 2010 22:00 Hello Dmitry, On Sat, Mar 20, 2010 at 12:20:07PM -0700, Dmitry Torokhov wrote: > Hi Wolfram, > > On Sat, Mar 20, 2010 at 03:12:47PM +0100, Wolfram Sang wrote: > > Fix I2C-drivers which missed setting clientdata to NULL before freeing the > > structure it points to. Also fix drivers which do this _after_ the structure > > was freed already. > > > > I am not sure if setting clientdata to NULL before freeing the data is > that important; we really want to be sure that we don't leave clientdata > dangling when we finish unbinding the driver. If there are another > thread the change will not really help the problem of accessing > non-existing client data. I agree it won't matter in practice. Jean and I concluded it is better style, though (http://comments.gmane.org/gmane.linux.drivers.i2c/5392). Still, I will not insist and leave it up the subsystem maintainers, of course. I also won't check for that case again in the future. > I will apply lm8323 portion of the patch but leave qt2160 as is. Thanks! Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
From: Wolfram Sang on 30 Mar 2010 08:40
Hi Dmitry, > On Sat, Mar 20, 2010 at 03:12:47PM +0100, Wolfram Sang wrote: > > Fix I2C-drivers which missed setting clientdata to NULL before freeing the > > structure it points to. Also fix drivers which do this _after_ the structure > > was freed already. > > > > I am not sure if setting clientdata to NULL before freeing the data is > that important; we really want to be sure that we don't leave clientdata > dangling when we finish unbinding the driver. If there are another > thread the change will not really help the problem of accessing > non-existing client data. > > I will apply lm8323 portion of the patch but leave qt2160 as is. We reached an agreement that a) setting the clientdata-pointer to NULL should be done in the i2c-core [1] and b) how to do it. Based on that, I will do the modification of the i2c-core for 2.6.34 (as it fixes the dangling pointers) and then create one single patch removing the then superflous calls to i2c_set_clientdata for 2.6.35 (as it is a cleanup). As you already applied the above patch to your branch: you don't have to revert, we will fix it for you in the next round. Sorry for the detour! Kind regards, Wolfram [1] http://thread.gmane.org/gmane.linux.drivers.i2c/5674/focus=5729 -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | |