Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-18 Thread Nayna
On 10/14/2016 12:21 AM, Nayna wrote: > > > On 10/01/2016 05:31 PM, Jarkko Sakkinen wrote: >> On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: >>> Currently, the securityfs pseudo files for obtaining the firmware >>> event log are created whether the event log properties exist or not.

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-13 Thread Nayna
On 10/01/2016 05:31 PM, Jarkko Sakkinen wrote: > On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: >> Currently, the securityfs pseudo files for obtaining the firmware >> event log are created whether the event log properties exist or not. >> This patch creates ascii and bios

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-11 Thread Nayna
On 10/12/2016 01:45 AM, Jason Gunthorpe wrote: > On Wed, Oct 12, 2016 at 12:41:05AM +0530, Nayna wrote: > >> Yeah, I actually tried this today. >> And on call of securityfs_remove(), release() gets called for the >> opened > > Are you saying securityfs_remove somehow causes a synchronous call to

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-11 Thread Jason Gunthorpe
On Wed, Oct 12, 2016 at 12:41:05AM +0530, Nayna wrote: > Yeah, I actually tried this today. > And on call of securityfs_remove(), release() gets called for the > opened Are you saying securityfs_remove somehow causes a synchronous call to release? How does that come about? > There are actually

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-11 Thread Jason Gunthorpe
On Mon, Oct 10, 2016 at 09:43:05AM +0530, Nayna wrote: > > > On 10/10/2016 08:51 AM, Jason Gunthorpe wrote: > >On Mon, Oct 10, 2016 at 07:23:33AM +0530, Nayna wrote: > > > >>And we pass this as private data to i_node in tpm_bios_log_setup. > > > >>So, we are referring chip as

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-09 Thread Nayna
On 10/10/2016 08:51 AM, Jason Gunthorpe wrote: > On Mon, Oct 10, 2016 at 07:23:33AM +0530, Nayna wrote: > >> And we pass this as private data to i_node in tpm_bios_log_setup. > >> So, we are referring chip as i_node->i_private->chip. > > That probably works, but you can't use the i_private =

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-09 Thread Jason Gunthorpe
On Mon, Oct 10, 2016 at 07:23:33AM +0530, Nayna wrote: > And we pass this as private data to i_node in tpm_bios_log_setup. > So, we are referring chip as i_node->i_private->chip. That probably works, but you can't use the i_private = NULL scheme I outlined with that. Jason

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-09 Thread Jason Gunthorpe
On Sun, Oct 09, 2016 at 09:47:08AM +0530, Nayna wrote: > >>+ const struct tpm_securityfs_data *sfs_data = > >>+ (const struct tpm_securityfs_data *)inode->i_private; > >>+ const struct seq_operations *seqops = sfs_data->seqops; > > > >You need a get_device(>dev) here, and the

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-08 Thread Nayna
On 10/03/2016 10:44 PM, Jason Gunthorpe wrote: > On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: >> @@ -323,34 +315,19 @@ static int tpm_bios_measurements_open(struct inode >> *inode, >> struct file *file) >> { >> int err; >> -

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Nayna
On 10/07/2016 01:40 AM, Jason Gunthorpe wrote: > On Fri, Oct 07, 2016 at 01:26:45AM +0530, Nayna wrote: > >> - there is no kref increment during eventlog fops or seq_ops operations. >> - fops and seq ops are parsing over memory buffer. fops->open() returns >> after giving the memory buffer(log)

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Jason Gunthorpe
On Fri, Oct 07, 2016 at 01:41:29AM +0530, Nayna wrote: > Are we trying to say that, once the teardown() is started, no more opening > of files are allowed, even if they are visible ? Yes. > But if open() has happened first, and then teardown(), in that case private > data is already passed to

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Nayna
On 10/04/2016 10:42 PM, Jason Gunthorpe wrote: > On Tue, Oct 04, 2016 at 08:26:51AM +0300, Jarkko Sakkinen wrote: >> On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: >>> On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: >>> > Sort of, the typical race is

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Jason Gunthorpe
On Fri, Oct 07, 2016 at 01:28:47AM +0530, Nayna wrote: > >fops->open() > > securityfs_remove() > > kref_put(chip) > > kfree(chip) > >kref_get(data->chip.kref) > > I didn't understand which kref_get() are we

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Jason Gunthorpe
On Fri, Oct 07, 2016 at 01:26:45AM +0530, Nayna wrote: > - there is no kref increment during eventlog fops or seq_ops operations. > - fops and seq ops are parsing over memory buffer. fops->open() returns > after giving the memory buffer(log) to seq->open(). And, seq ops on reading > of log memory

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-06 Thread Nayna
On 10/03/2016 10:05 PM, Jason Gunthorpe wrote: > On Mon, Oct 03, 2016 at 03:35:23PM +0300, Jarkko Sakkinen wrote: > The scheme you suggested is also way off the mark for how fops works, fops->close has no relation to the needed duration for 'data', the duration is related to

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-05 Thread Jarkko Sakkinen
On Tue, Oct 04, 2016 at 11:12:31AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 04, 2016 at 08:26:51AM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: > > > On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > > > > > > > Sort of,

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-04 Thread Jason Gunthorpe
On Tue, Oct 04, 2016 at 08:26:51AM +0300, Jarkko Sakkinen wrote: > On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: > > On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > > > > > Sort of, the typical race is broadly > > > > > > > > CPU0

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jarkko Sakkinen
On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: > On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > > > Sort of, the typical race is broadly > > > > > > CPU0 CPU1 > > > > > > fops->open() > > >

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jason Gunthorpe
On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > Sort of, the typical race is broadly > > > > CPU0 CPU1 > > > > fops->open() > > securityfs_remove() > > kref_put(chip) > >

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jason Gunthorpe
On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > @@ -323,34 +315,19 @@ static int tpm_bios_measurements_open(struct inode > *inode, > struct file *file) > { > int err; > - struct tpm_bios_log *log; > struct seq_file *seq; > -

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jason Gunthorpe
On Mon, Oct 03, 2016 at 03:35:23PM +0300, Jarkko Sakkinen wrote: > > > The scheme you suggested is also way off the mark for how fops works, > > > fops->close has no relation to the needed duration for 'data', the > > > duration is related to securityfs_remove. > > > > Right, the above would not

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jarkko Sakkinen
On Mon, Oct 03, 2016 at 03:20:13PM +0300, Jarkko Sakkinen wrote: > On Sun, Oct 02, 2016 at 03:25:51PM -0600, Jason Gunthorpe wrote: > > On Sat, Oct 01, 2016 at 10:32:39PM +0300, Jarkko Sakkinen wrote: > > > > chip held for the duration of any fops. Can open() start after the > > > > kref is

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-03 Thread Jarkko Sakkinen
On Sun, Oct 02, 2016 at 03:25:51PM -0600, Jason Gunthorpe wrote: > On Sat, Oct 01, 2016 at 10:32:39PM +0300, Jarkko Sakkinen wrote: > > > chip held for the duration of any fops. Can open() start after the > > > kref is dropped, etc. > > > > Why not make tpm_securityfs_data refcounted in order to

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jarkko Sakkinen
On Sat, Oct 01, 2016 at 10:32:39PM +0300, Jarkko Sakkinen wrote: > On Sat, Oct 01, 2016 at 10:54:36AM -0600, Jason Gunthorpe wrote: > > On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote: > > > > > > + struct tpmfs_data bin_sfs_data; > > > > + struct tpmfs_data

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jarkko Sakkinen
On Sat, Oct 01, 2016 at 10:54:36AM -0600, Jason Gunthorpe wrote: > On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote: > > > > + struct tpm_securityfs_data bin_sfs_data; > > > + struct tpm_securityfs_data ascii_sfs_data; > > > > I think this is otherwise right but the struct name is

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jason Gunthorpe
On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote: > > + struct tpm_securityfs_data bin_sfs_data; > > + struct tpm_securityfs_data ascii_sfs_data; > > I think this is otherwise right but the struct name is very clunky. > First of all it doesn't own the data and IMHO now it kind

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jarkko Sakkinen
On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote: > On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > > Currently, the securityfs pseudo files for obtaining the firmware > > event log are created whether the event log properties exist or not. > > This patch creates ascii

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jarkko Sakkinen
On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > Currently, the securityfs pseudo files for obtaining the firmware > event log are created whether the event log properties exist or not. > This patch creates ascii and bios measurements pseudo files > only if read_log() is successful.

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-01 Thread Jarkko Sakkinen
On Fri, Sep 30, 2016 at 08:42:13PM -0600, Jason Gunthorpe wrote: > On Fri, Sep 30, 2016 at 10:45:38PM +0300, Jarkko Sakkinen wrote: > > > Ok, this is interesting. What kind of refcounting bugs are related > > to existing approach? > > IIRC it was because the log was being processed in an fops

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-09-30 Thread Jason Gunthorpe
On Fri, Sep 30, 2016 at 10:45:38PM +0300, Jarkko Sakkinen wrote: > Ok, this is interesting. What kind of refcounting bugs are related > to existing approach? IIRC it was because the log was being processed in an fops open() callback, which itself was not properly serialized against chip

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-09-30 Thread Jarkko Sakkinen
On Fri, Sep 30, 2016 at 01:11:12PM -0600, Jason Gunthorpe wrote: > On Fri, Sep 30, 2016 at 09:57:43PM +0300, Jarkko Sakkinen wrote: > > On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > > > Currently, the securityfs pseudo files for obtaining the firmware > > > event log are created

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-09-30 Thread Jason Gunthorpe
On Fri, Sep 30, 2016 at 09:57:43PM +0300, Jarkko Sakkinen wrote: > On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > > Currently, the securityfs pseudo files for obtaining the firmware > > event log are created whether the event log properties exist or not. > > This patch creates ascii

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-09-30 Thread Jarkko Sakkinen
On Wed, Sep 28, 2016 at 04:34:37AM -0400, Nayna Jain wrote: > Currently, the securityfs pseudo files for obtaining the firmware > event log are created whether the event log properties exist or not. > This patch creates ascii and bios measurements pseudo files > only if read_log() is successful. >