Messages in this thread | | | Subject | Re: [PATCH 1/4] TPM: update char dev BKL pushdown | From | Rajiv Andrade <> | Date | Wed, 24 Sep 2008 10:14:40 -0300 |
| |
On Tue, 2008-09-23 at 14:58 -0600, Jonathan Corbet wrote: > On Tue, 23 Sep 2008 14:19:26 -0300 > Rajiv Andrade <srajiv@linux.vnet.ibm.com> wrote: > > > + * It's assured that the chip will be opened just once, > > + * by the check of is_open variable, which is protected > > + * by driver_lock. > > Taking a look at the code, I'm convinced. BKL removal seems > appropriate. > > While I was in the neighborhood, though, something caught my eye: > > int tpm_release(struct inode *inode, struct file *file) > { > struct tpm_chip *chip = file->private_data; > > flush_scheduled_work(); > > Here you have waited until you've got nothing in the workqueue. > > spin_lock(&driver_lock); > file->private_data = NULL; > del_singleshot_timer_sync(&chip->user_read_timer); > > But, until you get here, your timer could have resubmitted a job into > the workqueue - job which could run after you've freed "chip" and > forgotten all about it. I think you need either a "don't resubmit" flag, > or you need to delete the timer first. > > jon
Yes, like in tpm_read(), the timer must be deleted before flush_scheduled_work(). Since it's a fix to a new issue, I'm going to submit a another patch.
Thanks, Rajiv Andrade IBM Linux Technology Center Security Development
| |