From 86dc9693145bc3b2c21d2bc6a2563376ba8b15ff Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Fri, 23 Feb 2024 20:05:45 +0100 Subject: selinux: fix lsm_get_self_attr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selinux_getselfattr() doesn't properly initialize the string pointer it passes to selinux_lsm_getattr() which can cause a problem when an attribute hasn't been explicitly set; selinux_lsm_getattr() returns 0/success, but does not set or initialize the string label/attribute. Failure to properly initialize the string causes problems later in selinux_getselfattr() when the function attempts to kfree() the string. Cc: Casey Schaufler Fixes: 762c934317e6 ("SELinux: Add selfattr hooks") Suggested-by: Paul Moore [PM: description changes as discussed in the thread] Signed-off-by: Mickaël Salaün Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a6bf90ace84c..338b023a8c3e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6559,7 +6559,7 @@ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, size_t *size, u32 flags) { int rc; - char *val; + char *val = NULL; int val_len; val_len = selinux_lsm_getattr(attr, current, &val); -- cgit From 6d2fb472ea9ea27f765f10ba65ec73d30f6b7977 Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Fri, 23 Feb 2024 20:05:46 +0100 Subject: apparmor: fix lsm_get_self_attr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In apparmor_getselfattr() when an invalid AppArmor attribute is requested, or a value hasn't been explicitly set for the requested attribute, the label passed to aa_put_label() is not properly initialized which can cause problems when the pointer value is non-NULL and AppArmor attempts to drop a reference on the bogus label object. Cc: Casey Schaufler Cc: John Johansen Fixes: 223981db9baf ("AppArmor: Add selfattr hooks") Signed-off-by: Mickaël Salaün Reviewed-by: Paul Moore [PM: description changes as discussed with MS] Signed-off-by: Paul Moore --- security/apparmor/lsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 7717354ce095..63df97418b46 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -782,7 +782,7 @@ static int apparmor_getselfattr(unsigned int attr, struct lsm_ctx __user *lx, int error = -ENOENT; struct aa_task_ctx *ctx = task_ctx(current); struct aa_label *label = NULL; - char *value; + char *value = NULL; switch (attr) { case LSM_ATTR_CURRENT: -- cgit From d9818b3e906a0ee1ab02ea79e74a2f755fc5461a Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Mon, 19 Feb 2024 20:03:45 +0100 Subject: landlock: Fix asymmetric private inodes referring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When linking or renaming a file, if only one of the source or destination directory is backed by an S_PRIVATE inode, then the related set of layer masks would be used as uninitialized by is_access_to_paths_allowed(). This would result to indeterministic access for one side instead of always being allowed. This bug could only be triggered with a mounted filesystem containing both S_PRIVATE and !S_PRIVATE inodes, which doesn't seem possible. The collect_domain_accesses() calls return early if is_nouser_or_private() returns false, which means that the directory's superblock has SB_NOUSER or its inode has S_PRIVATE. Because rename or link actions are only allowed on the same mounted filesystem, the superblock is always the same for both source and destination directories. However, it might be possible in theory to have an S_PRIVATE parent source inode with an !S_PRIVATE parent destination inode, or vice versa. To make sure this case is not an issue, explicitly initialized both set of layer masks to 0, which means to allow all actions on the related side. If at least on side has !S_PRIVATE, then collect_domain_accesses() and is_access_to_paths_allowed() check for the required access rights. Cc: Arnd Bergmann Cc: Christian Brauner Cc: Günther Noack Cc: Jann Horn Cc: Shervin Oloumi Cc: stable@vger.kernel.org Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER") Link: https://lore.kernel.org/r/20240219190345.2928627-1-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index fc520a06f9af..0171f7eb6ee1 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -737,8 +737,8 @@ static int current_check_refer_path(struct dentry *const old_dentry, bool allow_parent1, allow_parent2; access_mask_t access_request_parent1, access_request_parent2; struct path mnt_dir; - layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS], - layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS]; + layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {}, + layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {}; if (!dom) return 0; -- cgit