Re: [PATCH 2/3] ser_gigaset: fix deallocation of platform device structure

[Paul Bolle]

On wo, 2015-12-09 at 12:10 +0100, Tilman Schmidt wrote:
> Am 09.12.2015 um 00:12 schrieb Paul Bolle:

> > So what does setting
> > cs->hw.ser->dev.dev.driver_data to NULL just before freeing it buy
> > us?
> 
> We're freeing cs->hw.ser, not cs->hw.ser->dev.
> Clearing the reference to cs from the device structure before freeing 
> cs guards against possible use-after-free.
> 
> > > +	kfree(cs->hw.ser);
> > > +	cs->hw.ser = NULL;
> > 
> > I might be missing something, but what does setting this to NULL buy 
> > us here?
> 
> Just defensive programming. Guarding against possible use-after-free 
> or double-free.

I'm inclined to think this is not the best way to guard against such
nasty bugs. But then again, I'm only a few months into my shift of
looking after the gigaset drivers and haven't had to track down such
bugs yet. But I'd be surprised if many other drivers do it that way and
think this is a job for (tree wide) debugging tools. But, whatever the
merits of our views, we can defer this discussion to some future date.
See below.

> I'm a big fan of one change per patch. If we also want to modify the
> moved code then that should be done in a separate patch. It makes
> bisecting so much easier. Same reason why I separated out patch 3/3.


Fair enough.


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/