Re: [PATCH] integrity: keep the integrity state of open files up to date

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

On Mon, 2019-05-06 at 15:53 +0300, Janne Karhunen wrote:
> From: Janne Karhunen <Janne.Karhunen@gmail.com>
> 
> When a file is open for writing, kernel crash or power outage
> is guaranteed to corrupt the inode integrity state leading to
> file appraisal failure on the subsequent boot. Add some basic
> infrastructure to keep the integrity measurements up to date as
> the files are written to.
> 
> Core file operations (open, close, sync, msync, truncate)
> update the measurement immediately. In order to maintain
> sufficient write performance for writes, add a latency tunable
> delayed work workqueue for computing the re-measurements.

Would renaming or deleting the file affect the wq?

> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
> Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>

Good, by only touching the "collected" iint status, re-measuring/re-
appraising files shouldn't be affected.

As I don't I have a test environment for testing this sort of change,
once the patches are ready, please Cc other interested parties.
 Probably some of the embedded mailing lists, yocto, and Patrick Ohly.

<snip>

> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index dc12fbcf484c..796147dbde37 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -20,6 +20,8 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask);
>  extern void ima_post_create_tmpfile(struct inode *inode);
>  extern void ima_file_free(struct file *file);
> +extern void ima_file_update(struct file *file);
> +extern void ima_delayed_update(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
>  extern int ima_load_data(enum kernel_load_data_id id);
>  extern int ima_read_file(struct file *file, enum kernel_read_file_id id);

Instead of using ifdef's before calling these functions, define the
associated stub functions as well.

 
> +/**
> + * ima_delayed_update - add a file to delayed update list
> + * @file: pointer to file structure being updated
> + */
> +void ima_delayed_update(struct file *file)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct integrity_iint_cache *iint;
> +	unsigned long blocks;
> +	unsigned long msecs;
> +	bool creq;
> +
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return;
> +
> +	if (iint->ima_work.file)
> +		return;
> +
> +	/* Slow down the samping rate per the file size */
> +	blocks = inode->i_size / SZ_1M + 1;
> +	msecs = blocks * IMA_LATENCY_INCREMENT;
> +	if (msecs > CONFIG_IMA_HASH_LATENCY_CEILING)
> +		msecs = CONFIG_IMA_HASH_LATENCY_CEILING;
> +
> +	get_file(file);
> +	iint->ima_work.file = file;
> +	INIT_DELAYED_WORK(&iint->ima_work.work, ima_delayed_update_handler);
> +
> +	creq = queue_delayed_work(ima_update_wq,
> +				  &iint->ima_work.work,
> +				  msecs_to_jiffies(msecs));
> +	if (creq == false) {
> +		iint->ima_work.file = NULL;
> +		fput(file);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(ima_delayed_update);

Does this need to be exported?

> +
> +/**
> + * ima_file_update - update the file measurement
> + * @file: pointer to file structure being updated
> + */
> +void ima_file_update(struct file *file)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct integrity_iint_cache *iint;
> +	bool should_measure = true;
> +	u64 i_version;
> +
> +	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
> +		return;
> +
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return;
> +
> +	mutex_lock(&iint->mutex);
> +	clear_bit(IMA_UPDATE_XATTR, &iint->atomic_flags);
> +	if (IS_I_VERSION(inode)) {
> +		i_version = inode_query_iversion(inode);
> +		if (i_version == iint->version)
> +			should_measure = false;
> +	}
> +	if (should_measure) {
> +		iint->flags &= ~IMA_COLLECTED;
> +		ima_update_xattr(iint, file);
> +	}
> +	mutex_unlock(&iint->mutex);
> +}
> +EXPORT_SYMBOL_GPL(ima_file_update);

And here?

Mimi