Re: [PATCH] Open a new file instance if no read permissions on files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21:01 06/10, Mimi Zohar wrote:
> On Fri, 2018-10-05 at 16:42 -0500, Goldwyn Rodrigues wrote:
> > Open a new file instance as opposed to changing file->f_mode when
> > the file is not readable.
> > 
> > This is done to accomodate overlayfs stacked file operations change. The
> > real struct file is hidden behind the overlays struct file. So, any
> > file->f_mode manipulations are not reflected on the real struct file.
> > Open the file again, read andcalculate the hash.
> > 
> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
> > 
> > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> > index 7e7e7e7c250a..3848cf208792 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -210,7 +210,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
> >  {
> >  	loff_t i_size, offset;
> >  	char *rbuf[2] = { NULL, };
> > -	int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
> > +	int rc, rbuf_len, active = 0, ahash_rc = 0;
> >  	struct ahash_request *req;
> >  	struct scatterlist sg[1];
> >  	struct crypto_wait wait;
> > @@ -257,11 +257,6 @@ static int ima_calc_file_hash_atfm(struct file *file,
> >  					  &rbuf_size[1], 0);
> >  	}
> >  
> > -	if (!(file->f_mode & FMODE_READ)) {
> > -		file->f_mode |= FMODE_READ;
> > -		read = 1;
> > -	}
> > -
> >  	for (offset = 0; offset < i_size; offset += rbuf_len) {
> >  		if (!rbuf[1] && offset) {
> >  			/* Not using two buffers, and it is not the first
> > @@ -300,8 +295,6 @@ static int ima_calc_file_hash_atfm(struct file *file,
> >  	/* wait for the last update request to complete */
> >  	rc = ahash_wait(ahash_rc, &wait);
> >  out3:
> > -	if (read)
> > -		file->f_mode &= ~FMODE_READ;
> >  	ima_free_pages(rbuf[0], rbuf_size[0]);
> >  	ima_free_pages(rbuf[1], rbuf_size[1]);
> >  out2:
> > @@ -336,7 +329,7 @@ static int ima_calc_file_hash_tfm(struct file *file,
> >  {
> >  	loff_t i_size, offset = 0;
> >  	char *rbuf;
> > -	int rc, read = 0;
> > +	int rc;
> >  	SHASH_DESC_ON_STACK(shash, tfm);
> >  
> >  	shash->tfm = tfm;
> > @@ -357,11 +350,6 @@ static int ima_calc_file_hash_tfm(struct file *file,
> >  	if (!rbuf)
> >  		return -ENOMEM;
> >  
> > -	if (!(file->f_mode & FMODE_READ)) {
> > -		file->f_mode |= FMODE_READ;
> > -		read = 1;
> > -	}
> > -
> >  	while (offset < i_size) {
> >  		int rbuf_len;
> >  
> > @@ -378,8 +366,6 @@ static int ima_calc_file_hash_tfm(struct file *file,
> >  		if (rc)
> >  			break;
> >  	}
> > -	if (read)
> > -		file->f_mode &= ~FMODE_READ;
> >  	kfree(rbuf);
> >  out:
> >  	if (!rc)
> > @@ -419,7 +405,7 @@ static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
> >  int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> >  {
> >  	loff_t i_size;
> > -	int rc;
> > +	int read = 0, rc;
> >  
> >  	/*
> >  	 * For consistency, fail file's opened with the O_DIRECT flag on
> > @@ -431,15 +417,29 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (!(file->f_mode & FMODE_READ)) {
> > +		struct file *f;
> 
> I would define "struct file *f = file" above, at the beginning of
> function, and "free(f)" below, without modifying "file".

I suppose you mean fput(f).
Okay, if it makes code more understandable.

> 
> > +		int flags = file->f_flags & ~(O_WRONLY | O_APPEND | O_TRUNC | O_CREAT | O_NOCTTY | O_EXCL);
> 
> Doesn't O_RDONLY need to be added?

No. O_RDONLY is zero. But I think I should add it for readability. The
compiler will optimize it eventually.

> Please fix the line length.
> 
> 
> > +		f = dentry_open(&file->f_path, flags, file->f_cred);
> > +		if (IS_ERR(f))
> > +			return PTR_ERR(f);
> > +		read = 1;
> > +		file = f;
> 
> With the above change, no need to modify "file".
> 
> Mimi
> 
> > +	}
> > +
> >  	i_size = i_size_read(file_inode(file));
> >  
> >  	if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
> >  		rc = ima_calc_file_ahash(file, hash);
> >  		if (!rc)
> > -			return 0;
> > +			goto out;
> >  	}
> >  
> > -	return ima_calc_file_shash(file, hash);
> > +	rc = ima_calc_file_shash(file, hash);
> > +out:
> > +	if (read)
> > +		fput(file);
> > +	return rc;
> >  }
> >  
> >  /*
> > 
> 

-- 
Goldwyn



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux