linux-kernel - Re: [PATCH 1/4] TPM: update char dev BKL pushdown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1222262080.24276.32.camel@blackbox>
Date:	Wed, 24 Sep 2008 10:14:40 -0300
From:	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
To:	Jonathan Corbet <corbet@....net>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	jmoriss@...ei.org, serue@...ux.vnet.ibm.com,
	zohar@...ux.vnet.ibm.com, safford@...son.ibm.com,
	paulmck@...ux.vnet.ibm.com, debora@...ux.vnet.ibm.com
Subject: Re: [PATCH 1/4] TPM: update char dev BKL pushdown

On Tue, 2008-09-23 at 14:58 -0600, Jonathan Corbet wrote:
> On Tue, 23 Sep 2008 14:19:26 -0300
> Rajiv Andrade <srajiv@...ux.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/