lkml.org 
[lkml]   [2020]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] ima: verify mprotect change is consistent with mmap policy
From
Date
On 5/4/20 2:17 PM, Mimi Zohar wrote:

Hi Mimi,

> +int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
> +{
> + struct ima_template_desc *template;
> + struct inode *inode;
> + int result = 0;
> + int action;
> + u32 secid;
> + int pcr;
> +
> + if (vma->vm_file && (prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {

Just a suggestion:
Maybe you could do the negative of the above check and return, so that
the block within the if statement doesn't have to be indented.

> + inode = file_inode(vma->vm_file);
> +
> + security_task_getsecid(current, &secid);
> + action = ima_get_action(inode, current_cred(), secid, MAY_EXEC,
> + MMAP_CHECK, &pcr, &template, 0);
> +
> + if (action & IMA_APPRAISE_SUBMASK)
> + result = -EPERM;
> +
> + if ((action & IMA_APPRAISE_SUBMASK) || (action & IMA_MEASURE)) {

action is checked for IMA_APPRAISE_SUBMASK bits in the previous if
statement. Does it need to be checked again in the above if statement?

> + struct file *file = vma->vm_file;
> + char *pathbuf = NULL;
> + const char *pathname;
> + char filename[NAME_MAX];
> +
> + pathname = ima_d_path(&file->f_path, &pathbuf,
> + filename);
> + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
> + pathname, "collect_data",
> + "failed-mprotect", result, 0);
> +
> + if (pathbuf)
> + __putname(pathbuf);
> + }
> + }
> + return result;
> +}

thanks,
-lakshmi

\
 
 \ /
  Last update: 2020-05-05 00:53    [W:2.304 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site