From 59454ecf490b1b00ded4a59710a0b738b371ed60 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Tue, 27 Feb 2007 08:42:00 +0000 Subject: [PATCH] Add mount-consistent-d_cache.diff and file-handle-ops.diff. Update to last link-subset-check.diff proposal. Minor rediffs to get rid of offsets. --- .../for-mainline/constrained-to-confined.diff | 6 +- .../for-mainline/d_namespace_path.diff | 2 +- .../for-mainline/file-handle-ops.diff | 163 ++++++++++++++++++ .../fix-cleanup-aa_register-2.diff | 12 +- .../fix-deleted-revalidation.diff | 2 +- .../fix-make-parent-point-to-itself.diff | 2 +- .../fix-tell-files-from-dirs.diff | 2 +- .../for-mainline/link-subset-check.diff | 86 ++++++--- .../mount-consistent-d_cache.diff | 59 +++++++ .../for-mainline/put_task_context_safely.diff | 2 +- kernel-patches/for-mainline/series | 2 + 11 files changed, 301 insertions(+), 37 deletions(-) create mode 100644 kernel-patches/for-mainline/file-handle-ops.diff create mode 100644 kernel-patches/for-mainline/mount-consistent-d_cache.diff diff --git a/kernel-patches/for-mainline/constrained-to-confined.diff b/kernel-patches/for-mainline/constrained-to-confined.diff index dbc583eca..6fce45bb8 100644 --- a/kernel-patches/for-mainline/constrained-to-confined.diff +++ b/kernel-patches/for-mainline/constrained-to-confined.diff @@ -59,7 +59,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -831,7 +831,7 @@ aa_register_find(struct aa_profile *prof +@@ -802,7 +802,7 @@ aa_register_find(struct aa_profile *prof } } else { /* Only way we can get into this code is if task @@ -68,7 +68,7 @@ Index: b/security/apparmor/main.c */ AA_DEBUG("%s: No profile found for exec image %s\n", __FUNCTION__, -@@ -867,7 +867,7 @@ repeat: +@@ -838,7 +838,7 @@ repeat: if (profile) { complain = PROFILE_COMPLAIN(profile); @@ -77,7 +77,7 @@ Index: b/security/apparmor/main.c * mandatory to load new profile */ exec_mode = aa_match(profile->file_rules, filename); -@@ -881,8 +881,8 @@ repeat: +@@ -852,8 +852,8 @@ repeat: /* nothing to be done here */ goto cleanup; diff --git a/kernel-patches/for-mainline/d_namespace_path.diff b/kernel-patches/for-mainline/d_namespace_path.diff index b72dae72e..3bcc64fe1 100644 --- a/kernel-patches/for-mainline/d_namespace_path.diff +++ b/kernel-patches/for-mainline/d_namespace_path.diff @@ -26,7 +26,7 @@ Index: b/fs/dcache.c + struct dentry *root, struct vfsmount *rootmnt, + char *buffer, int buflen, int fail_deleted) { - int namelen, is_slash; + int namelen, is_slash, vfsmount_locked = 0; Index: b/fs/namespace.c =================================================================== diff --git a/kernel-patches/for-mainline/file-handle-ops.diff b/kernel-patches/for-mainline/file-handle-ops.diff new file mode 100644 index 000000000..bab8faa5b --- /dev/null +++ b/kernel-patches/for-mainline/file-handle-ops.diff @@ -0,0 +1,163 @@ +Index: b/include/linux/fs.h +=================================================================== +--- a/include/linux/fs.h ++++ b/include/linux/fs.h +@@ -349,6 +349,9 @@ struct iattr { + * Not an attribute, but an auxilary info for filesystems wanting to + * implement an ftruncate() like method. NOTE: filesystem should + * check for (ia_valid & ATTR_FILE), and not for (ia_file != NULL). ++ * ++ * The LSM hooks also use this to distinguish operations on a file ++ * descriptors from operations on pathnames. + */ + struct file *ia_file; + }; +Index: b/fs/open.c +=================================================================== +--- a/fs/open.c ++++ b/fs/open.c +@@ -520,6 +520,8 @@ asmlinkage long sys_fchmod(unsigned int + mode = inode->i_mode; + newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO); + newattrs.ia_valid = ATTR_MODE | ATTR_CTIME; ++ newattrs.ia_valid |= ATTR_FILE; ++ newattrs.ia_file = file; + err = notify_change(dentry, file->f_path.mnt, &newattrs); + mutex_unlock(&inode->i_mutex); + +@@ -570,7 +572,7 @@ asmlinkage long sys_chmod(const char __u + } + + static int chown_common(struct dentry * dentry, struct vfsmount *mnt, +- uid_t user, gid_t group) ++ uid_t user, gid_t group, struct file *file) + { + struct inode * inode; + int error; +@@ -598,6 +600,10 @@ static int chown_common(struct dentry * + } + if (!S_ISDIR(inode->i_mode)) + newattrs.ia_valid |= ATTR_KILL_SUID|ATTR_KILL_SGID; ++ if (file) { ++ newattrs.ia_file = file; ++ newattrs.ia_valid |= ATTR_FILE; ++ } + mutex_lock(&inode->i_mutex); + error = notify_change(dentry, mnt, &newattrs); + mutex_unlock(&inode->i_mutex); +@@ -613,7 +619,7 @@ asmlinkage long sys_chown(const char __u + error = user_path_walk(filename, &nd); + if (error) + goto out; +- error = chown_common(nd.dentry, nd.mnt, user, group); ++ error = chown_common(nd.dentry, nd.mnt, user, group, NULL); + path_release(&nd); + out: + return error; +@@ -633,7 +639,7 @@ asmlinkage long sys_fchownat(int dfd, co + error = __user_walk_fd(dfd, filename, follow, &nd); + if (error) + goto out; +- error = chown_common(nd.dentry, nd.mnt, user, group); ++ error = chown_common(nd.dentry, nd.mnt, user, group, NULL); + path_release(&nd); + out: + return error; +@@ -647,7 +653,7 @@ asmlinkage long sys_lchown(const char __ + error = user_path_walk_link(filename, &nd); + if (error) + goto out; +- error = chown_common(nd.dentry, nd.mnt, user, group); ++ error = chown_common(nd.dentry, nd.mnt, user, group, NULL); + path_release(&nd); + out: + return error; +@@ -666,7 +672,7 @@ asmlinkage long sys_fchown(unsigned int + + dentry = file->f_path.dentry; + audit_inode(NULL, dentry->d_inode); +- error = chown_common(dentry, file->f_path.mnt, user, group); ++ error = chown_common(dentry, file->f_path.mnt, user, group, file); + fput(file); + out: + return error; +Index: b/security/apparmor/lsm.c +=================================================================== +--- a/security/apparmor/lsm.c ++++ b/security/apparmor/lsm.c +@@ -82,7 +82,7 @@ static int apparmor_ptrace(struct task_s + * under the rules that the kernel normally permits if the two + * processes are running under the same profile, but then we + * would probably have to reject profile changes for processes +- * that are being ptraces as well as for processes ptracing ++ * that are being ptraced as well as for processes ptracing + * others. + */ + +Index: b/security/apparmor/main.c +=================================================================== +--- a/security/apparmor/main.c ++++ b/security/apparmor/main.c +@@ -190,7 +190,7 @@ static inline void aa_put_name_buffer(ch + + static int aa_perm_dentry(struct aa_profile *profile, struct dentry *dentry, + struct vfsmount *mnt, struct aa_audit *sa, int mask, +- int is_dir) ++ int is_dir, int accessed_through_fd) + { + char *buffer = NULL; + int denied_mask, error; +@@ -198,8 +198,11 @@ static int aa_perm_dentry(struct aa_prof + sa->name = aa_get_name(dentry, mnt, &buffer, is_dir); + + if (IS_ERR(sa->name)) { +- /* deleted files are given a pass on permission checks */ +- if (PTR_ERR(sa->name) == -ENOENT) ++ /* ++ * deleted files are given a pass on permission checks when ++ * accessed through a file descriptor. ++ */ ++ if (PTR_ERR(sa->name) == -ENOENT && accessed_through_fd) + denied_mask = 0; + else + denied_mask = PTR_ERR(sa->name); +@@ -538,7 +541,8 @@ int aa_attr(struct aa_profile *profile, + sa.gfp_mask = GFP_KERNEL; + + error = aa_perm_dentry(profile, dentry, mnt, &sa, MAY_WRITE, +- S_ISDIR(dentry->d_inode->i_mode)); ++ S_ISDIR(dentry->d_inode->i_mode), ++ iattr->ia_valid & ATTR_FILE); + + return error; + } +@@ -566,7 +570,8 @@ int aa_perm_xattr(struct aa_profile *pro + sa.gfp_mask = GFP_KERNEL; + + error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, +- S_ISDIR(dentry->d_inode->i_mode)); ++ S_ISDIR(dentry->d_inode->i_mode), ++ 0 /* FIXME */); + + return error; + } +@@ -608,7 +613,8 @@ int aa_perm(struct aa_profile *profile, + sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, +- inode && S_ISDIR(inode->i_mode)); ++ inode && S_ISDIR(inode->i_mode), ++ 0 /* FIXME */); + + out: + return error; +@@ -636,7 +642,8 @@ int aa_perm_dir(struct aa_profile *profi + sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + +- return aa_perm_dentry(profile, dentry, mnt, &sa, mask, 1); ++ return aa_perm_dentry(profile, dentry, mnt, &sa, mask, 1, ++ 0 /* FIXME */); + } + + /** diff --git a/kernel-patches/for-mainline/fix-cleanup-aa_register-2.diff b/kernel-patches/for-mainline/fix-cleanup-aa_register-2.diff index d9c097cac..ed97b9fc6 100644 --- a/kernel-patches/for-mainline/fix-cleanup-aa_register-2.diff +++ b/kernel-patches/for-mainline/fix-cleanup-aa_register-2.diff @@ -2,7 +2,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -797,16 +797,17 @@ repeat: +@@ -768,16 +768,17 @@ repeat: } static struct aa_profile * @@ -26,7 +26,7 @@ Index: b/security/apparmor/main.c if (complain) { LOG_HINT(profile, GFP_KERNEL, HINT_MANDPROF, "image=%s pid=%d profile=%s active=%s\n", -@@ -832,7 +833,7 @@ aa_register_find(const char *name, int m +@@ -803,7 +804,7 @@ aa_register_find(const char *name, int m __FUNCTION__, name); } @@ -35,7 +35,7 @@ Index: b/security/apparmor/main.c } /** -@@ -847,7 +848,7 @@ int aa_register(struct linux_binprm *bpr +@@ -818,7 +819,7 @@ int aa_register(struct linux_binprm *bpr char *filename, *buffer = NULL; struct file *filp = bprm->file; struct aa_profile *profile, *old_profile, *new_profile = NULL; @@ -44,7 +44,7 @@ Index: b/security/apparmor/main.c AA_DEBUG("%s\n", __FUNCTION__); -@@ -889,7 +890,8 @@ repeat: +@@ -860,7 +861,8 @@ repeat: AA_DEBUG("%s: PROFILE %s\n", __FUNCTION__, filename); @@ -54,7 +54,7 @@ Index: b/security/apparmor/main.c complain); break; -@@ -926,7 +928,7 @@ repeat: +@@ -897,7 +899,7 @@ repeat: } } else { /* Unconfined task, load profile if it exists */ @@ -63,7 +63,7 @@ Index: b/security/apparmor/main.c if (new_profile == NULL) goto cleanup; } -@@ -955,7 +957,7 @@ repeat: +@@ -926,7 +928,7 @@ repeat: * Cases 2 and 3 are marked as requiring secure exec * (unless policy specified "unsafe exec") */ diff --git a/kernel-patches/for-mainline/fix-deleted-revalidation.diff b/kernel-patches/for-mainline/fix-deleted-revalidation.diff index 1ccd64c9d..177c9fc8b 100644 --- a/kernel-patches/for-mainline/fix-deleted-revalidation.diff +++ b/kernel-patches/for-mainline/fix-deleted-revalidation.diff @@ -2,7 +2,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -227,7 +227,11 @@ static int aa_perm_dentry(struct aa_prof +@@ -198,7 +198,11 @@ static int aa_perm_dentry(struct aa_prof sa->name = aa_get_name(dentry, mnt, &buffer, is_dir); if (IS_ERR(sa->name)) { diff --git a/kernel-patches/for-mainline/fix-make-parent-point-to-itself.diff b/kernel-patches/for-mainline/fix-make-parent-point-to-itself.diff index ae01060cd..3e657d76e 100644 --- a/kernel-patches/for-mainline/fix-make-parent-point-to-itself.diff +++ b/kernel-patches/for-mainline/fix-make-parent-point-to-itself.diff @@ -2,7 +2,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -1137,7 +1137,7 @@ repeat: +@@ -1108,7 +1108,7 @@ repeat: goto out; } diff --git a/kernel-patches/for-mainline/fix-tell-files-from-dirs.diff b/kernel-patches/for-mainline/fix-tell-files-from-dirs.diff index b98e03bc5..9fa5fd950 100644 --- a/kernel-patches/for-mainline/fix-tell-files-from-dirs.diff +++ b/kernel-patches/for-mainline/fix-tell-files-from-dirs.diff @@ -2,7 +2,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -633,7 +633,7 @@ int aa_perm(struct aa_profile *profile, +@@ -604,7 +604,7 @@ int aa_perm(struct aa_profile *profile, sa.flags = 0; sa.gfp_mask = GFP_KERNEL; error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, diff --git a/kernel-patches/for-mainline/link-subset-check.diff b/kernel-patches/for-mainline/link-subset-check.diff index 5f10ebbcd..ccaf4d008 100644 --- a/kernel-patches/for-mainline/link-subset-check.diff +++ b/kernel-patches/for-mainline/link-subset-check.diff @@ -2,45 +2,85 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -138,21 +138,31 @@ static int aa_file_denied(struct aa_prof +@@ -62,32 +62,6 @@ static inline int aa_taskattr_access(con + return strcmp(end, "/attr/current") == 0; + } + +-/** +- * aa_file_mode - get full mode for file entry from profile +- * @profile: profile +- * @name: filename +- */ +-static inline int aa_file_mode(struct aa_profile *profile, const char *name) +-{ +- int perms = 0; +- +- AA_DEBUG("%s: %s\n", __FUNCTION__, name); +- if (!name) { +- AA_DEBUG("%s: no name\n", __FUNCTION__); +- goto out; +- } +- +- if (!profile) { +- AA_DEBUG("%s: no profile\n", __FUNCTION__); +- goto out; +- } +- +- perms = aa_match(profile->file_rules, name); +- +-out: +- return perms; +-} +- + static inline void aa_permerror2result(int perm_result, struct aa_audit *sa) + { + if (perm_result == 0) { /* success */ +@@ -132,27 +106,34 @@ static int aa_file_denied(struct aa_prof + * @link: name of link being created + * @target: name of target to be linked to + * +- * Return %0 on success, or else the permissions in @mask that the +- * profile denies. ++ * Return %0 on success, or else the permissions that the profile denies. + */ static int aa_link_denied(struct aa_profile *profile, const char *link, const char *target) { - int l_mode, t_mode, ret = -EPERM; -+ int l_mode, t_mode; - - l_mode = aa_file_mode(profile, link); +- +- l_mode = aa_file_mode(profile, link); - if (l_mode & AA_MAY_LINK) { - /* mask off link bit */ - l_mode &= ~AA_MAY_LINK; -+ t_mode = aa_file_mode(profile, target); ++ int l_mode, t_mode; - t_mode = aa_file_mode(profile, target); - t_mode &= ~AA_MAY_LINK; -+ /** -+ * If the process does not have the link permission for the link, or -+ * the read, write, and execute permissions of the link are not a -+ * subset of those of the target, or any of the remaining permissions -+ * (which are all execute related) differ between the link and the -+ * target, deny the access. -+ * -+ * FIXME: There currenly is no way to report which other permissions -+ * the process would need to have in l_mode, or which permissions in -+ * l_mode would have to be removed in order for the operation to -+ * succeed. -+ */ ++ l_mode = aa_match(profile->file_rules, link); ++ t_mode = aa_match(profile->file_rules, target); - if (l_mode == t_mode) - ret = 0; - } -+ if (!(l_mode & AA_MAY_LINK) || -+ (~t_mode & l_mode & (MAY_READ | MAY_WRITE | MAY_EXEC)) || -+ ((l_mode & ~(MAY_READ | MAY_WRITE | MAY_EXEC | AA_MAY_LINK)) != -+ (t_mode & ~(MAY_READ | MAY_WRITE | MAY_EXEC | AA_MAY_LINK)))) -+ return AA_MAY_LINK; ++ /** ++ * Allow to link if we have permission to rename, or we are allowed to ++ * link and to read the target. ++ * ++ * FIXME: We could warn if we have link write permission, but not ++ * target write permission -- this could be considered a form of ++ * permission elevation. ++ */ ++ if (((l_mode & MAY_WRITE) && (t_mode & (MAY_READ | MAY_WRITE))) || ++ ((l_mode & AA_MAY_LINK) && (t_mode & MAY_READ))) ++ return 0; - return ret; -+ return 0; ++ /** ++ * FIXME: There currenly is no way to report which permissions ++ * we expect in t_mode, so linking could fail even after learning ++ * the required l_mode. ++ */ ++ return AA_MAY_LINK; } static char *aa_get_name(struct dentry *dentry, struct vfsmount *mnt, diff --git a/kernel-patches/for-mainline/mount-consistent-d_cache.diff b/kernel-patches/for-mainline/mount-consistent-d_cache.diff new file mode 100644 index 000000000..09c275c75 --- /dev/null +++ b/kernel-patches/for-mainline/mount-consistent-d_cache.diff @@ -0,0 +1,59 @@ +Make d_path() consistent across mount operations + +Right now, the path that __d_path() computes can become slightly +inconsistent when it races with mount operations: it grabs the +vfsmount_lock when traversing mount points, but immediately drops the +lock again, only to re-grab it when it reaches the next mount point. +The result is that the filename computed is not always consisent, and +the file may never have had that name. (This is extremely unlikely, but +still possible.) + +This can easily be fixed by grabbing the vfsmount_lock when the first +mount point is reached, and holding onto it until the d_cache lookup is +completed. As a bonus, this makes __d_path() slightly more efficient +when traversing a number of mount points. + +Signed-off-by: Andreas Gruenbacher + +Index: b/fs/dcache.c +=================================================================== +--- a/fs/dcache.c ++++ b/fs/dcache.c +@@ -1754,7 +1754,7 @@ static char *__d_path(struct dentry *den + struct dentry *root, struct vfsmount *rootmnt, + char *buffer, int buflen, int fail_deleted) + { +- int namelen, is_slash; ++ int namelen, is_slash, vfsmount_locked = 0; + + if (buflen < 2) + return ERR_PTR(-ENAMETOOLONG); +@@ -1777,14 +1777,14 @@ static char *__d_path(struct dentry *den + struct dentry * parent; + + if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) { +- spin_lock(&vfsmount_lock); +- if (vfsmnt->mnt_parent == vfsmnt) { +- spin_unlock(&vfsmount_lock); +- goto global_root; ++ if (!vfsmount_locked) { ++ spin_lock(&vfsmount_lock); ++ vfsmount_locked = 1; + } ++ if (vfsmnt->mnt_parent == vfsmnt) ++ goto global_root; + dentry = vfsmnt->mnt_mountpoint; + vfsmnt = vfsmnt->mnt_parent; +- spin_unlock(&vfsmount_lock); + continue; + } + parent = dentry->d_parent; +@@ -1803,6 +1803,8 @@ static char *__d_path(struct dentry *den + *--buffer = '/'; + + out: ++ if (vfsmount_locked) ++ spin_unlock(&vfsmount_lock); + spin_unlock(&dcache_lock); + return buffer; + diff --git a/kernel-patches/for-mainline/put_task_context_safely.diff b/kernel-patches/for-mainline/put_task_context_safely.diff index d5ce8ce56..9c959a801 100644 --- a/kernel-patches/for-mainline/put_task_context_safely.diff +++ b/kernel-patches/for-mainline/put_task_context_safely.diff @@ -2,7 +2,7 @@ Index: b/security/apparmor/main.c =================================================================== --- a/security/apparmor/main.c +++ b/security/apparmor/main.c -@@ -1028,8 +1028,11 @@ repeat: +@@ -999,8 +999,11 @@ repeat: list_del(&cxt->list); unlock_profile(profile); aa_put_profile(profile); diff --git a/kernel-patches/for-mainline/series b/kernel-patches/for-mainline/series index e2c5af07c..2034f31bd 100644 --- a/kernel-patches/for-mainline/series +++ b/kernel-patches/for-mainline/series @@ -29,6 +29,7 @@ vfs-removexattr.diff security-removexattr.diff d_path-lazy-unmounts.diff no-unreachable-paths.diff +mount-consistent-d_cache.diff # security_chroot.diff apparmor-audit.diff apparmor.diff @@ -95,3 +96,4 @@ aafs-perms.diff put_task_context_safely.diff apparmor-interface-revision.diff constrained-to-confined.diff +file-handle-ops.diff