Re: Kernel hangs after DVB patch from July 2021 with Hauppauge WinTV dualHD

[Pavel Skripkin <paskripkin@gmail.com>]

On 12/18/21 23:33, Robert Schlabbach wrote:
>> Such patch should actually be fixing a use-after-free on disconnect.
> 
> I wonder if it's the right thing to do, though, because if you look into the em28xx_close_extension() function in em28xx-core.c:
> 
> 		if (ops->fini) {
> 			if (dev->dev_next)
> 				ops->fini(dev->dev_next);
> 			ops->fini(dev);
> 		}
> 
> So doing this in em28xx_cards.c:
> 
> 	em28xx_close_extension(dev);
> 
> 	if (dev->dev_next) {
> 		em28xx_close_extension(dev->dev_next);
> 		em28xx_release_resources(dev->dev_next);
> 	}
> 
> will end up calling ops->fini() twice on dev->dev_next, no matter in which order you put the calls.
> 

I don't see any problem in calling ->fini() twice. I see only 4 ->fini() 
variants and of them have guards against double free.

I've checked out if there possible kref unbalance, but ->init() is also 
called 2 times: em28xx_register_extension() and em28xx_init_extension().


> So it looks prone to double-free, but at least em28xx_dvb_fini() in em28xx_dvb.c guards against that by NULLing the dev->dvb pointer after free and checking the pointer at entry.
> 
> Still, there are redundant calls here. I think a decision should be made whether dev->dev_next is taken care of in em28xx-core.c or in em28xx-cards.c, and the code then be made consistent accordingly.
> 

Agree. Some kind of refactoring should be done :)


With regards,
Pavel Skripkin