On Wed, 10 May 2006, Andrew de Quincey wrote: > Hi - updated the http://linuxtv.org/hg/~quincy/v4l-dvb-attach tree with the > last round of ideas for this. This looks pretty good. I think there is a problem with the way ops->release is used after ops->release() is called. It looks like with most front-ends, ops is a pointer into the front-end private state. The FE release method frees this memory, then continues to access it when it sets ops->release to NULL. Then dvb_unregister_frontend() will look at fe->ops->release after the memory for fe->ops has been freed. The comment /* fe is invalid now */ should stay right after release() is called, inside the loop that is using fe. I don't think is hard to solve. The setting and checking of ops->release doesn't seem to be necessary at all. The loop in dvb_unregister_fontend() can be removed, by making chained modules responsible for putting the module that was in front of them. dvb_unregister_fontend() will look like this: { struct module *m; [other stuff...] m = fe->ops->release(fe); /* fe is invalid now */ if(m) module_put(m); [rest of code...] } A front-end release function would look like this: static struct module *or51132_release(struct dvb_frontend* fe) { struct or51132_state* state = fe->demodulator_priv; kfree(state); return THIS_MODULE; } A release function for a chaining module would look like this: static stuct module *lnbp21_release(struct dvb_frontend *fe) { struct lnbp21 *lnbp21 = (struct lnbp21 *) fe->misc_priv; struct module *m; /* Restore old release function, _before_ lnbp12 is freed */ fe->ops->release = lnbp12->release_chain; kfree(lnbp21); /* fe->misc_priv is invalid now */ m = fe->ops->release(fe); /* fe is invalid now */ if(m) module_put(m); return THIS_MODULE; } _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
