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

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

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 of implies
> of owning.

These are passed in here:

> >  	chip->bios_dir[chip->bios_dir_count] =
> >  	    securityfs_create_file("ascii_bios_measurements",
> >  				   S_IRUSR | S_IRGRP, chip->bios_dir[0],
> > -				   (void *)&tpm_ascii_b_measurments_seqops,
> > +				   (void *)&chip->ascii_sfs_data,
> >  				   &tpm_bios_measurements_ops);

And the argument to securityfs_create_file is called 'data'..

The key question with these patches is if all the locking is done
right and we have the correct lifetime model now.

Eg how much does securityfs_remove serialize and is the kref on the
chip held for the duration of any fops. Can open() start after the
kref is dropped, etc.

Otherwise this scheme isn't good enough either :/

I haven't looked in detail at that topic yet.. Maybe Nayna can explain
what is expected here. Would be excellend to get someone from security to
review this lifetime model.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot