Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,65 @@ Subject: [PATCH] Revert "selinux: fix overlayfs mmap() and mprotect() access
checks"

This reverts commit d844702198395d3f80222777030f69db6be6b709.

[vyaghras:
Ported to work with the changes introduced in
d127c7e1f3e1231d8ac8b2738e3f00bea12afe74 and
9d99b770e7b67b00bdef9005b928aad13e1d679b.
]
Signed-off-by: Shikha vyaghra <vyaghras@amazon.com>
---
security/selinux/hooks.c | 248 ++++++++----------------------
security/selinux/include/objsec.h | 11 --
2 files changed, 64 insertions(+), 195 deletions(-)
security/selinux/hooks.c | 388 ++++++------------------------
security/selinux/include/objsec.h | 19 --
2 files changed, 76 insertions(+), 331 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index af188e19c130..5efd444674ef 100644
index eef75bde6..5efd44467 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1739,72 +1739,49 @@ static inline int file_path_has_perm(const struct cred *cred,
@@ -1665,32 +1665,26 @@ static int cred_has_capability(const struct cred *cred,
return rc;
}

-/*
- * Check whether a SID has a particular permission to an inode. The 'adp'
- * parameter is optional and allows other audit data to be passed (e.g. the
- * dentry).
- */
-static int inode_sid_has_perm(u32 sid, struct inode *inode, u32 perms,
- struct common_audit_data *adp)
+/* Check whether a task has a particular permission to an inode.
+ The 'adp' parameter is optional and allows other audit
+ data to be passed (e.g. the dentry). */
+static int inode_has_perm(const struct cred *cred,
+ struct inode *inode,
+ u32 perms,
+ struct common_audit_data *adp)
{
struct inode_security_struct *isec;
+ u32 sid;

if (unlikely(IS_PRIVATE(inode)))
return 0;

+ sid = cred_sid(cred);
isec = selinux_inode(inode);

return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp);
}

-static int inode_has_perm(const struct cred *cred,
- struct inode *inode,
- u32 perms,
- struct common_audit_data *adp)
-{
- return inode_sid_has_perm(cred_sid(cred), inode, perms, adp);
-}
-
/* Same as inode_has_perm, but pass explicit audit data containing
the dentry to help the auditing code to more easily generate the
pathname if needed. */
@@ -1745,72 +1739,49 @@ static inline int file_path_has_perm(const struct cred *cred,
static int bpf_fd_pass(const struct file *file, u32 sid);
#endif

Expand Down Expand Up @@ -92,11 +141,11 @@ index af188e19c130..5efd444674ef 100644
+ rc = 0;
if (av)
- return inode_has_perm(cred, inode, av, &ad);
+ rc = inode_has_perm(cred, inode, av, &ad);

-
- return 0;
-}
-
+ rc = inode_has_perm(cred, inode, av, &ad);

-/* Check whether a task can use an open file descriptor to
- access an inode in a given way. Check access to the
- descriptor itself, and then use dentry_has_perm to
Expand All @@ -114,28 +163,132 @@ index af188e19c130..5efd444674ef 100644
}

/*
@@ -3822,17 +3799,6 @@ static int selinux_file_alloc_security(struct file *file)
@@ -3828,74 +3799,6 @@ static int selinux_file_alloc_security(struct file *file)
return 0;
}

-static inline u32 selinux_file_user_sid(const struct file *file)
-{
- if (unlikely(file->f_mode & FMODE_BACKING))
- return selinux_backing_file(file)->uf_sid;
- return selinux_file(file)->sid;
-}
-
-static int selinux_backing_file_alloc(struct file *backing_file,
- const struct file *user_file)
-{
- struct backing_file_security_struct *bfsec;
- const struct backing_file_security_struct *ubfsec;
- struct backing_file_security_layer *layer;
- u32 i;
-
- bfsec = selinux_backing_file(backing_file);
- bfsec->uf_sid = selinux_file(user_file)->sid;
- bfsec->uf_sid = selinux_file_user_sid(user_file);
- if (!(user_file->f_mode & FMODE_BACKING))
- return 0;
-
- ubfsec = selinux_backing_file(user_file);
- /* a wrapped count would make kmalloc_array() return ZERO_SIZE_PTR */
- if (unlikely(ubfsec->layer_count == U32_MAX))
- return -EOVERFLOW;
-
- /*
- * The final VMA only retains the lowest backing file, so record the
- * whole chain here rather than in the mmap hook, where concurrent
- * mappings would have to be serialized. Size it dynamically: erofs
- * inode sharing adds a backing file without bumping s_stack_depth.
- */
- bfsec->layers = kmalloc_array(ubfsec->layer_count + 1,
- sizeof(*bfsec->layers), GFP_KERNEL);
- if (!bfsec->layers)
- return -ENOMEM;
-
- for (i = 0; i < ubfsec->layer_count; i++) {
- layer = &bfsec->layers[i];
- *layer = ubfsec->layers[i];
- path_get(&layer->path);
- }
-
- /* f_path, not file_user_path(): this layer, not the top-level file */
- layer = &bfsec->layers[i];
- layer->path = user_file->f_path;
- layer->mounter_sid = cred_sid(user_file->f_cred);
- layer->fd_sid = selinux_file(user_file)->sid;
- path_get(&layer->path);
- bfsec->layer_count = ubfsec->layer_count + 1;
-
- return 0;
-}
-
-static void selinux_backing_file_free(struct file *backing_file)
-{
- struct backing_file_security_struct *bfsec;
-
- /* security_backing_file_free() may be called twice after an error */
- if (!backing_file_security(backing_file))
- return;
-
- bfsec = selinux_backing_file(backing_file);
- while (bfsec->layer_count)
- path_put(&bfsec->layers[--bfsec->layer_count].path);
- kfree(bfsec->layers);
- bfsec->layers = NULL;
-}
-
/*
* Check whether a task has the ioctl permission and cmd
* operation to an inode.
@@ -3950,57 +3916,42 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,
@@ -4013,100 +3916,42 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,

static int default_noexec __ro_after_init;

-static u32 file_map_prot_to_av(unsigned long prot, bool shared)
-{
- u32 av = FILE__READ;
-
- if (shared && (prot & PROT_WRITE))
- av |= FILE__WRITE;
- if (prot & PROT_EXEC)
- av |= FILE__EXECUTE;
-
- return av;
-}
-
-static int backing_mounters_has_perm(const struct file *file, u32 av)
-{
- const struct backing_file_security_struct *bfsec;
- const struct backing_file_security_layer *layer;
- struct common_audit_data ad;
- struct inode *inode;
- u32 i;
- int rc;
-
- if (WARN_ON_ONCE(!(file->f_mode & FMODE_BACKING)))
- return -EIO;
-
- bfsec = selinux_backing_file(file);
- for (i = 0; i < bfsec->layer_count; i++) {
- layer = &bfsec->layers[i];
- inode = d_inode(layer->path.dentry);
-
- ad.type = LSM_AUDIT_DATA_PATH;
- ad.u.path = layer->path;
-
- if (layer->mounter_sid != layer->fd_sid) {
- rc = avc_has_perm(layer->mounter_sid, layer->fd_sid,
- SECCLASS_FD, FD__USE, &ad);
- if (rc)
- return rc;
- }
-
- rc = inode_sid_has_perm(layer->mounter_sid, inode, av, &ad);
- if (rc)
- return rc;
- }
-
- return 0;
-}
-
-static int __file_map_prot_check(const struct file *file, unsigned long prot,
- bool shared, bool mounter_check,
- bool bf_user_file)
Expand Down Expand Up @@ -182,37 +335,36 @@ index af188e19c130..5efd444674ef 100644
if (file) {
- const struct cred *cred = mounter_check ?
- file->f_cred : current_cred();
- /* "read" always possible, "write" only if shared */
+ /* read access is always possible with a mapping */
u32 av = FILE__READ;
- if (shared && prot_write)
+
+ u32 av = FILE__READ;

- return __file_has_perm(cred, file,
- file_map_prot_to_av(prot, shared),
- bf_user_file);
- }
+ /* write access only matters if the mapping is shared */
+ if (shared && (prot & PROT_WRITE))
av |= FILE__WRITE;
- if (prot_exec)
+
+ if (prot & PROT_EXEC)
av |= FILE__EXECUTE;

- return __file_has_perm(cred, file, av, bf_user_file);
+ return file_has_perm(cred, file, av);
}
+ av |= FILE__WRITE;

- return 0;
-}
-
+ if (prot & PROT_EXEC)
+ av |= FILE__EXECUTE;

-static inline int file_map_prot_check(const struct file *file,
- unsigned long prot, bool shared,
- bool mounter_check)
-{
- return __file_map_prot_check(file, prot, shared, mounter_check, false);
+ return file_has_perm(cred, file, av);
+ }
+
+error:
+ return rc;
}

static int selinux_mmap_addr(unsigned long addr)
@@ -4016,84 +3967,36 @@ static int selinux_mmap_addr(unsigned long addr)
@@ -4122,85 +3967,36 @@ static int selinux_mmap_addr(unsigned long addr)
return rc;
}

Expand Down Expand Up @@ -295,6 +447,7 @@ index af188e19c130..5efd444674ef 100644
- int rc;
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
- u32 av;
- const struct file *file = vma->vm_file;
- bool backing_file;
- bool shared = vma->vm_flags & VM_SHARED;
Expand All @@ -308,7 +461,7 @@ index af188e19c130..5efd444674ef 100644
/*
* We don't use the vma_is_initial_heap() helper as it has
* a history of problems and is currently broken on systems
@@ -4107,15 +4010,11 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
@@ -4214,15 +4010,11 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
vma->vm_end <= vma->vm_mm->brk) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
Expand All @@ -326,7 +479,7 @@ index af188e19c130..5efd444674ef 100644
/*
* We are making executable a file mapping that has
* had some COW done. Since pages might have been
@@ -4123,29 +4022,13 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
@@ -4230,37 +4022,13 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
* modified content. This typically should only
* occur for text relocations.
*/
Expand All @@ -335,6 +488,10 @@ index af188e19c130..5efd444674ef 100644
- if (rc)
- return rc;
- if (backing_file) {
- rc = backing_mounters_has_perm(file,
- FILE__EXECMOD);
- if (rc)
- return rc;
- rc = file_has_perm(file->f_cred, file,
- FILE__EXECMOD);
- if (rc)
Expand All @@ -348,6 +505,10 @@ index af188e19c130..5efd444674ef 100644
- if (rc)
- return rc;
- if (backing_file) {
- av = file_map_prot_to_av(prot, shared);
- rc = backing_mounters_has_perm(file, av);
- if (rc)
- return rc;
- rc = file_map_prot_check(file, prot, shared, true);
if (rc)
return rc;
Expand All @@ -358,19 +519,20 @@ index af188e19c130..5efd444674ef 100644
}

static int selinux_file_lock(struct file *file, unsigned int cmd)
@@ -7275,7 +7158,6 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
@@ -7390,7 +7158,6 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
.lbs_cred = sizeof(struct cred_security_struct),
.lbs_task = sizeof(struct task_security_struct),
.lbs_file = sizeof(struct file_security_struct),
- .lbs_backing_file = sizeof(struct backing_file_security_struct),
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
.lbs_key = sizeof(struct key_security_struct),
@@ -7499,11 +7381,9 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
@@ -7614,12 +7381,9 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {

LSM_HOOK_INIT(file_permission, selinux_file_permission),
LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
- LSM_HOOK_INIT(backing_file_alloc, selinux_backing_file_alloc),
- LSM_HOOK_INIT(backing_file_free, selinux_backing_file_free),
LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
LSM_HOOK_INIT(file_ioctl_compat, selinux_file_ioctl_compat),
LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
Expand All @@ -379,21 +541,29 @@ index af188e19c130..5efd444674ef 100644
LSM_HOOK_INIT(file_mprotect, selinux_file_mprotect),
LSM_HOOK_INIT(file_lock, selinux_file_lock),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index fcb46793898f..816fde5a5896 100644
index bbb04f26b..816fde5a5 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -86,10 +86,6 @@ struct file_security_struct {
@@ -86,18 +86,6 @@ struct file_security_struct {
u32 pseqno; /* Policy seqno at the time of file open */
};

-struct backing_file_security_layer {
- struct path path; /* this layer's real path */
- u32 mounter_sid; /* SID of the mounter that opened it */
- u32 fd_sid; /* SID of its open file description */
-};
-
-struct backing_file_security_struct {
- u32 uf_sid; /* associated user file fsec->sid */
- u32 uf_sid; /* top-level user file fsec->sid */
- u32 layer_count; /* number of intermediate backing files */
- struct backing_file_security_layer *layers;
-};
-
struct superblock_security_struct {
u32 sid; /* SID of file system superblock */
u32 def_sid; /* default SID for labeling */
@@ -194,13 +190,6 @@ static inline struct file_security_struct *selinux_file(const struct file *file)
@@ -202,13 +190,6 @@ static inline struct file_security_struct *selinux_file(const struct file *file)
return file->f_security + selinux_blob_sizes.lbs_file;
}

Expand All @@ -408,5 +578,5 @@ index fcb46793898f..816fde5a5896 100644
selinux_inode(const struct inode *inode)
{
--
2.47.0
2.50.1

Loading
Loading