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.

This commit is contained in:
Andreas Gruenbacher 2007-02-27 08:42:00 +00:00
parent 3bc6bf34ab
commit 59454ecf49
11 changed files with 301 additions and 37 deletions

View file

@ -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;

View file

@ -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
===================================================================

View file

@ -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 */);
}
/**

View file

@ -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")
*/

View file

@ -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)) {

View file

@ -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;
}

View file

@ -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,

View file

@ -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,

View file

@ -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 <agruen@suse.de>
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;

View file

@ -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);

View file

@ -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