On 2018-01-04 18:19, Michael Ira Krufky wrote: > On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@xxxxxxxxxxxxxxxx> wrote: >> If release is part of frontend ops then it is called in the >> course of dvb_frontend_detach. The process also decrements >> the module usage count. The problem is if the lgdt3306a >> driver is reached via i2c_new_device, then when it is >> eventually destroyed remove is called, which further >> decrements the module usage count to negative. After this >> occurs the driver is in a bad state and no longer works. >> Also fixed by NULLing out the release callback is a double >> kfree of state, which introduces arbitrary oopses/GPF. >> This problem is only currently reachable via the em28xx driver. >> >> On disconnect of Hauppauge SoloHD before: >> >> lsmod | grep lgdt3306a >> lgdt3306a 28672 -1 >> i2c_mux 16384 1 lgdt3306a >> >> On disconnect of Hauppauge SoloHD after: >> >> lsmod | grep lgdt3306a >> lgdt3306a 28672 0 >> i2c_mux 16384 1 lgdt3306a >> >> Signed-off-by: Brad Love <brad@xxxxxxxxxxxxxxxx> >> --- >> drivers/media/dvb-frontends/lgdt3306a.c | 1 + >> 1 file changed, 1 insertion(+) >> > Brad, > > We won't be able to apply this one. The symptom that you're trying to > fix is indicative of some other problem, probably in the em28xx > driver. NULL'ing the release callback is not the right thing to do. > > -Mike Krufky Hey Mike, Redacting this patch to deal with individually. I will separately send to the list an example of another bridge driver which exhibits the same issue, when using the lgdt3306a driver similarly. I don't think this is solely an em28xx issue. I will also send a patch fixing only the double free, as that seems to be most important. Cheers, Brad >> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c >> index 6356815..d2477ed 100644 >> --- a/drivers/media/dvb-frontends/lgdt3306a.c >> +++ b/drivers/media/dvb-frontends/lgdt3306a.c >> @@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client, >> >> i2c_set_clientdata(client, fe->demodulator_priv); >> state = fe->demodulator_priv; >> + state->frontend.ops.release = NULL; >> >> /* create mux i2c adapter for tuner */ >> state->muxc = i2c_mux_alloc(client->adapter, &client->dev, >> -- >> 2.7.4 >>
