[RFC v2 5/9] S.A.R.A. WX Protection
flat view
From: Kees Cook <hidden>
Date: 2017-06-27 23:04:29
Also in:
linux-mm, lkml
On Thu, Jun 15, 2017 at 9:42 AM, Salvatore Mesoraca [off-list ref] wrote:
quoted hunk ↗ jump to hunk
+static int sara_check_vmflags(vm_flags_t vm_flags) +{ + u16 sara_wxp_flags = get_current_sara_wxp_flags(); + + if (sara_enabled && wxprot_enabled) { + if (sara_wxp_flags & SARA_WXP_WXORX && + vm_flags & VM_WRITE && + vm_flags & VM_EXEC) { + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) + pr_wxp("W^X"); + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) + return -EPERM; + } + if (sara_wxp_flags & SARA_WXP_MMAP && + (vm_flags & VM_EXEC || + (!(vm_flags & VM_MAYWRITE) && (vm_flags & VM_MAYEXEC))) && + get_current_sara_mmap_blocked()) { + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) + pr_wxp("executable mmap"); + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) + return -EPERM; + } + }
Given the subtle differences between these various if blocks (here and in the other hook), I think it would be nice to have some beefy comments here to describe specifically what's being checked (and why). It'll help others review this code, and help validate code against intent. I would also try to minimize the written code by creating a macro for a repeated pattern here:
quoted hunk ↗ jump to hunk
+ if ((sara_wxp_flags & SARA_WXP_VERBOSE)) + pr_wxp("mprotect on file mmap"); + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) + return -EACCES;
These four lines are repeated several times with only the const char *
and return value changing. Perhaps something like:
#define sara_return(err, msg) do { \
if ((sara_wxp_flags & SARA_WXP_VERBOSE)) \
pr_wxp(err); \
if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) \
return -err; \
} while (0)
Then each if block turns into something quite easier to parse:
if (sara_wxp_flags & SARA_WXP_WXORX &&
vm_flags & VM_WRITE &&
vm_flags & VM_EXEC)
sara_return(EPERM, "W^X");
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html