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

[Nayna <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]

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 = NULL scheme I
> outlined with that.

Why ? we are doing i_private = NULL during teardown to imply that chip 
unregister is in progress. and no more securityfs operations should be 
done. So, whether chip is NULL or securityfs_data is NULL, either should 
be ok. Isn't it ?

Below is the open() function with NULL check and get_device(). 
put_device() I will do in release()

static int tpm_bios_measurements_open(struct inode *inode,
                                             struct file *file)
{
         int err;
         struct seq_file *seq;
         struct tpm_securityfs_data *sfs_data;
         const struct seq_operations *seqops;
         struct tpm_chip *chip;

         inode_lock(inode);
         if (!inode->i_private) {  ---> This would be made NULL by teardown
                 inode_unlock(inode);
                 return -ENODEV;
         }
         sfs_data = (const struct tpm_securityfs_data *)inode->i_private;
         seqops = sfs_data->seqops;
         chip = sfs_data->chip;
         get_device(&chip->dev);
         /* now register seq file */
         err = seq_open(file, seqops);
         if (!err) {
                 seq = file->private_data;
                 seq->private = &chip->log;
         }
         inode_unlock(inode);

         return err;
}


Let me know if I am missing something basic.

Thanks & Regards,
   - Nayna


>
> Jason
>


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