On Thu, 2017-11-30 at 11:56 +0100, Roberto Sassu wrote:
> ima_rdwr_violation_check() retrieves the full path of a measured file by
> calling ima_d_path(). If process_measurement() calls this function, it
> reuses the pointer and passes it to the functions to measure/appraise/audit
> an accessed file.
>
> After commit bc15ed663e7e ("ima: fix ima_d_path() possible race with
> rename"), ima_d_path() first tries to retrieve the full path by calling
> d_absolute_path() and, if there is an error, copies the dentry name to the
> buffer passed as argument.
>
> However, ima_rdwr_violation_check() passes to ima_d_path() the pointer of a
> local variable. process_measurement() might be reusing the pointer to an
> area in the stack which may have been already overwritten after
> ima_rdwr_violation_check() returned.
>
> Correct this issue by passing to ima_rdwr_violation_check() the pointer of
> a buffer declared in process_measurement().
>
> Fixes: bc15ed663e7e ("ima: fix ima_d_path() possible race with rename")
> Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
Thanks!
> ---
> security/integrity/ima/ima_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 294b2fe69334..5a7321bc325c 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -84,10 +84,10 @@ static void ima_rdwr_violation_check(struct file *file,
> struct integrity_iint_cache *iint,
> int must_measure,
> char **pathbuf,
> - const char **pathname)
> + const char **pathname,
> + char *filename)
> {
> struct inode *inode = file_inode(file);
> - char filename[NAME_MAX];
> fmode_t mode = file->f_mode;
> bool send_tomtou = false, send_writers = false;
>
> @@ -205,7 +205,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>
> if (violation_check) {
> ima_rdwr_violation_check(file, iint, action & IMA_MEASURE,
> - &pathbuf, &pathname);
> + &pathbuf, &pathname, filename);
> if (!action) {
> rc = 0;
> goto out_free;
