Skip to content

Commit 9fe595f

Browse files
WOnder93pcmoore
authored andcommitted
selinux: fix incorrect execmem checks on overlayfs
The commit fixing the overlayfs mmap() and mprotect() access checks failed to skip the execmem check in __file_map_prot_check() for the case where the "mounter check" is being performed. This check should be performed only against the credentials of the task that is calling mmap()/mprotect(), since it doesn't pertain to the file itself, but rather just gates the ability of the calling task to get an executable memory mapping in general. The purpose of the "mounter check" is to guard against using an overlayfs mount to gain file access that would otherwise be denied to the mounter. For execmem this is not relevant, as there is no further file access granted based on it (notice that the file's context is not used as the target in the check), so checking it also against the mounter credentials would be incorrect. Fix this by passing a boolean to [__]file_map_prot_check() and selinux_mmap_file_common() that indicates if we are doing the "mounter check" and skiping the execmem check in that case. Since this boolean also indicates if we use current_cred() or the mounter cred as the subject, also remove the "cred" argument from these functions and determine it based on the boolean and the file struct. Cc: stable@vger.kernel.org Fixes: 82544d3 ("selinux: fix overlayfs mmap() and mprotect() access checks") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com> Signed-off-by: Paul Moore <paul@paul-moore.com>
1 parent a13c140 commit 9fe595f

1 file changed

Lines changed: 24 additions & 18 deletions

File tree

security/selinux/hooks.c

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3969,9 +3969,9 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,
39693969

39703970
static int default_noexec __ro_after_init;
39713971

3972-
static int __file_map_prot_check(const struct cred *cred,
3973-
const struct file *file, unsigned long prot,
3974-
bool shared, bool bf_user_file)
3972+
static int __file_map_prot_check(const struct file *file, unsigned long prot,
3973+
bool shared, bool mounter_check,
3974+
bool bf_user_file)
39753975
{
39763976
struct inode *inode = NULL;
39773977
bool prot_exec = prot & PROT_EXEC;
@@ -3984,10 +3984,10 @@ static int __file_map_prot_check(const struct cred *cred,
39843984
inode = file_inode(file);
39853985
}
39863986

3987-
if (default_noexec && prot_exec &&
3987+
if (!mounter_check && default_noexec && prot_exec &&
39883988
(!file || IS_PRIVATE(inode) || (!shared && prot_write))) {
39893989
int rc;
3990-
u32 sid = cred_sid(cred);
3990+
u32 sid = current_sid();
39913991

39923992
/*
39933993
* We are making executable an anonymous mapping or a private
@@ -4000,6 +4000,8 @@ static int __file_map_prot_check(const struct cred *cred,
40004000
}
40014001

40024002
if (file) {
4003+
const struct cred *cred = mounter_check ?
4004+
file->f_cred : current_cred();
40034005
/* "read" always possible, "write" only if shared */
40044006
u32 av = FILE__READ;
40054007
if (shared && prot_write)
@@ -4013,11 +4015,11 @@ static int __file_map_prot_check(const struct cred *cred,
40134015
return 0;
40144016
}
40154017

4016-
static inline int file_map_prot_check(const struct cred *cred,
4017-
const struct file *file,
4018-
unsigned long prot, bool shared)
4018+
static inline int file_map_prot_check(const struct file *file,
4019+
unsigned long prot, bool shared,
4020+
bool mounter_check)
40194021
{
4020-
return __file_map_prot_check(cred, file, prot, shared, false);
4022+
return __file_map_prot_check(file, prot, shared, mounter_check, false);
40214023
}
40224024

40234025
static int selinux_mmap_addr(unsigned long addr)
@@ -4033,12 +4035,14 @@ static int selinux_mmap_addr(unsigned long addr)
40334035
return rc;
40344036
}
40354037

4036-
static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
4037-
unsigned long prot, bool shared)
4038+
static int selinux_mmap_file_common(struct file *file, unsigned long prot,
4039+
bool shared, bool mounter_check)
40384040
{
40394041
if (file) {
40404042
int rc;
40414043
struct common_audit_data ad;
4044+
const struct cred *cred = mounter_check ?
4045+
file->f_cred : current_cred();
40424046

40434047
ad.type = LSM_AUDIT_DATA_FILE;
40444048
ad.u.file = file;
@@ -4047,15 +4051,16 @@ static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
40474051
return rc;
40484052
}
40494053

4050-
return file_map_prot_check(cred, file, prot, shared);
4054+
return file_map_prot_check(file, prot, shared, mounter_check);
40514055
}
40524056

40534057
static int selinux_mmap_file(struct file *file,
40544058
unsigned long reqprot __always_unused,
40554059
unsigned long prot, unsigned long flags)
40564060
{
4057-
return selinux_mmap_file_common(current_cred(), file, prot,
4058-
(flags & MAP_TYPE) == MAP_SHARED);
4061+
return selinux_mmap_file_common(file, prot,
4062+
(flags & MAP_TYPE) == MAP_SHARED,
4063+
false);
40594064
}
40604065

40614066
/**
@@ -4087,8 +4092,9 @@ static int selinux_mmap_backing_file(struct vm_area_struct *vma,
40874092
if (vma->vm_flags & VM_EXEC)
40884093
prot |= PROT_EXEC;
40894094

4090-
return selinux_mmap_file_common(backing_file->f_cred, backing_file,
4091-
prot, vma->vm_flags & VM_SHARED);
4095+
return selinux_mmap_file_common(backing_file, prot,
4096+
vma->vm_flags & VM_SHARED,
4097+
true);
40924098
}
40934099

40944100
static int selinux_file_mprotect(struct vm_area_struct *vma,
@@ -4149,11 +4155,11 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
41494155
}
41504156
}
41514157

4152-
rc = __file_map_prot_check(cred, file, prot, shared, backing_file);
4158+
rc = __file_map_prot_check(file, prot, shared, false, backing_file);
41534159
if (rc)
41544160
return rc;
41554161
if (backing_file) {
4156-
rc = file_map_prot_check(file->f_cred, file, prot, shared);
4162+
rc = file_map_prot_check(file, prot, shared, true);
41574163
if (rc)
41584164
return rc;
41594165
}

0 commit comments

Comments
 (0)