Re: [PATCH 2/3] ima: update the file measurement on truncate

[Mimi Zohar <zohar@linux.ibm.com>]

On Mon, 2019-09-02 at 12:45 +0300, Janne Karhunen wrote:
> Let IMA know when a file is being opened with truncate
> or truncated directly.
> 
> Depends on commit 72649b7862a7 ("ima: keep the integrity state of open files up to date")'
> 
> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
> Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
> ---
>  fs/namei.c | 5 ++++-
>  fs/open.c  | 3 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 209c51a5226c..0994fe26bef1 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3418,8 +3418,11 @@ static int do_last(struct nameidata *nd,
>  		goto out;
>  opened:
>  	error = ima_file_check(file, op->acc_mode);
> -	if (!error && will_truncate)
> +	if (!error && will_truncate) {
>  		error = handle_truncate(file);
> +		if (!error)
> +			ima_file_update(file);

Security and IMA hooks are normally named after the function.  For
example, there's a security hook named security_path_truncate() in
handle_truncate().  The new hook after the truncate would either be
named security_post_path_truncate() or ima_post_path_truncate().

> +	}
>  out:
>  	if (unlikely(error > 0)) {
>  		WARN_ON(1);
> diff --git a/fs/open.c b/fs/open.c
> index a59abe3c669a..98c2d4629371 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -63,6 +63,9 @@ int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
>  	/* Note any delegations or leases have already been broken: */
>  	ret = notify_change(dentry, &newattrs, NULL);
>  	inode_unlock(dentry->d_inode);
> +
> +	if (filp)
> +		ima_file_update(filp);
>  	return ret;
>  }
>  

do_truncate() is called from a number of places.  Are you sure that
the call to IMA should be in all of them?  security_path_truncate()
isn't in all of the callers.

Mimi