Messages in this thread | | | Subject | Re: [RFC PATCH] ima: verify mprotect change is consistent with mmap policy | From | Lakshmi Ramasubramanian <> | Date | Mon, 4 May 2020 15:51:47 -0700 |
| |
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
| |