| Message ID | 1515164233-2423-3-git-send-email-brad@nextdimension.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 05.01.2018 um 15:57 schrieb Brad Love: > Both lgdt33606a_release and lgdt3306a_remove kfree state, but _release is > called first, then _remove operates on states members before kfree'ing it. > This can lead to random oops/GPF/etc on USB disconnect. > lgdt3306a_release does nothing but the kfree. So the exact same effect can be archived by setting state->frontend.ops.release to NULL. This need to be done already at probe time I think. lgdt3306a_remove does this, but too late (after the call to release). Regards Matthias > Signed-off-by: Brad Love <brad@nextdimension.cc> > --- > drivers/media/dvb-frontends/lgdt3306a.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c > index d370671..3642e6e 100644 > --- a/drivers/media/dvb-frontends/lgdt3306a.c > +++ b/drivers/media/dvb-frontends/lgdt3306a.c > @@ -1768,7 +1768,13 @@ static void lgdt3306a_release(struct dvb_frontend *fe) > struct lgdt3306a_state *state = fe->demodulator_priv; > > dbg_info("\n"); > - kfree(state); > + > + /* > + * If state->muxc is not NULL, then we are an i2c device > + * and lgdt3306a_remove will clean up state > + */ > + if (!state->muxc) > + kfree(state); > } > > static const struct dvb_frontend_ops lgdt3306a_ops; >
On 2018-01-08 14:34, Matthias Schwarzott wrote: > Am 05.01.2018 um 15:57 schrieb Brad Love: >> Both lgdt33606a_release and lgdt3306a_remove kfree state, but _release is >> called first, then _remove operates on states members before kfree'ing it. >> This can lead to random oops/GPF/etc on USB disconnect. >> > lgdt3306a_release does nothing but the kfree. So the exact same effect > can be archived by setting state->frontend.ops.release to NULL. This > need to be done already at probe time I think. > lgdt3306a_remove does this, but too late (after the call to release). > > Regards > Matthias Hi Matthias, I agree. This was my rationale in the previous patch: /patch/46328 Both methods handle the issue. I thought the previous attempt was fairly clean, but it did not pass review, so I provided this solution. Cheers, Brad >> Signed-off-by: Brad Love <brad@nextdimension.cc> >> --- >> drivers/media/dvb-frontends/lgdt3306a.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c >> index d370671..3642e6e 100644 >> --- a/drivers/media/dvb-frontends/lgdt3306a.c >> +++ b/drivers/media/dvb-frontends/lgdt3306a.c >> @@ -1768,7 +1768,13 @@ static void lgdt3306a_release(struct dvb_frontend *fe) >> struct lgdt3306a_state *state = fe->demodulator_priv; >> >> dbg_info("\n"); >> - kfree(state); >> + >> + /* >> + * If state->muxc is not NULL, then we are an i2c device >> + * and lgdt3306a_remove will clean up state >> + */ >> + if (!state->muxc) >> + kfree(state); >> } >> >> static const struct dvb_frontend_ops lgdt3306a_ops; >>
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c index d370671..3642e6e 100644 --- a/drivers/media/dvb-frontends/lgdt3306a.c +++ b/drivers/media/dvb-frontends/lgdt3306a.c @@ -1768,7 +1768,13 @@ static void lgdt3306a_release(struct dvb_frontend *fe) struct lgdt3306a_state *state = fe->demodulator_priv; dbg_info("\n"); - kfree(state); + + /* + * If state->muxc is not NULL, then we are an i2c device + * and lgdt3306a_remove will clean up state + */ + if (!state->muxc) + kfree(state); } static const struct dvb_frontend_ops lgdt3306a_ops;
Both lgdt33606a_release and lgdt3306a_remove kfree state, but _release is called first, then _remove operates on states members before kfree'ing it. This can lead to random oops/GPF/etc on USB disconnect. Signed-off-by: Brad Love <brad@nextdimension.cc> --- drivers/media/dvb-frontends/lgdt3306a.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)