| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1193398252.4721.7.camel@localhost>
Date: Fri, 26 Oct 2007 13:30:52 +0200
From: Miklos Szeredi <mszeredi@...e.cz>
To: jjohansen@...e.de
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
Andreas Gruenbacher <agruen@...e.de>
Subject: Re: [AppArmor 32/45] Enable LSM hooks to distinguish operations on
file descriptors from operations on pathnames
On Thu, 2007-10-25 at 23:40 -0700, jjohansen@...e.de wrote:
> plain text document attachment (file-handle-ops.diff)
> Struct iattr already contains ia_file since commit cc4e69de from
> Miklos (which is related to commit befc649c). Use this to pass
> struct file down the setattr hooks. This allows LSMs to distinguish
> operations on file descriptors from operations on paths.
There's a slight problem (other than HCH not liking it) with this
approach of passing the open file in iattr: for special files, the
struct file pointer makes no sense to the filesystem, since it is always
opened by the generic functions.
This wasn't a problem with ftruncate(), because that one only works on
regular files, but fchmod/fchown/futimes will work on special files as
well, and the filesystem interpreting file->private_data could cause
nasty bugs.
So I think the correct solution (which was suggested by Trond and
others) is to define an f_op->fsetattr() method, which interested
filesystems can define.
Miklos
>
> Signed-off-by: Andreas Gruenbacher <agruen@...e.de>
> Signed-off-by: John Johansen <jjohansen@...e.de>
> Cc: Miklos Szeredi <mszeredi@...e.cz>
>
> ---
> fs/nfsd/vfs.c | 12 +++++++-----
> fs/open.c | 16 +++++++++++-----
> include/linux/fs.h | 3 +++
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -413,7 +413,7 @@ static ssize_t nfsd_getxattr(struct dent
> {
> ssize_t buflen;
>
> - buflen = vfs_getxattr(dentry, mnt, key, NULL, 0);
> + buflen = vfs_getxattr(dentry, mnt, key, NULL, 0, NULL);
> if (buflen <= 0)
> return buflen;
>
> @@ -421,7 +421,7 @@ static ssize_t nfsd_getxattr(struct dent
> if (!*buf)
> return -ENOMEM;
>
> - return vfs_getxattr(dentry, mnt, key, *buf, buflen);
> + return vfs_getxattr(dentry, mnt, key, *buf, buflen, NULL);
> }
> #endif
>
> @@ -447,7 +447,7 @@ set_nfsv4_acl_one(struct dentry *dentry,
> goto out;
> }
>
> - error = vfs_setxattr(dentry, mnt, key, buf, len, 0);
> + error = vfs_setxattr(dentry, mnt, key, buf, len, 0, NULL);
> out:
> kfree(buf);
> return error;
> @@ -2062,12 +2062,14 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
>
> mnt = fhp->fh_export->ex_mnt;
> if (size)
> - error = vfs_setxattr(fhp->fh_dentry, mnt, name, value, size,0);
> + error = vfs_setxattr(fhp->fh_dentry, mnt, name, value, size, 0,
> + NULL);
> else {
> if (!S_ISDIR(inode->i_mode) && type == ACL_TYPE_DEFAULT)
> error = 0;
> else {
> - error = vfs_removexattr(fhp->fh_dentry, mnt, name);
> + error = vfs_removexattr(fhp->fh_dentry, mnt, name,
> + NULL);
> if (error == -ENODATA)
> error = 0;
> }
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -594,6 +594,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);
>
> @@ -648,7 +650,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;
> @@ -674,6 +676,10 @@ static int chown_common(struct dentry *
> if (!S_ISDIR(inode->i_mode))
> newattrs.ia_valid |=
> ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
> + 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);
> @@ -692,7 +698,7 @@ asmlinkage long sys_chown(const char __u
> error = mnt_want_write(nd.mnt);
> if (error)
> goto out_release;
> - error = chown_common(nd.dentry, nd.mnt, user, group);
> + error = chown_common(nd.dentry, nd.mnt, user, group, NULL);
> mnt_drop_write(nd.mnt);
> out_release:
> path_release(&nd);
> @@ -717,7 +723,7 @@ asmlinkage long sys_fchownat(int dfd, co
> error = mnt_want_write(nd.mnt);
> if (error)
> goto out_release;
> - error = chown_common(nd.dentry, nd.mnt, user, group);
> + error = chown_common(nd.dentry, nd.mnt, user, group, NULL);
> mnt_drop_write(nd.mnt);
> out_release:
> path_release(&nd);
> @@ -736,7 +742,7 @@ asmlinkage long sys_lchown(const char __
> error = mnt_want_write(nd.mnt);
> if (error)
> goto out_release;
> - error = chown_common(nd.dentry, nd.mnt, user, group);
> + error = chown_common(nd.dentry, nd.mnt, user, group, NULL);
> mnt_drop_write(nd.mnt);
> out_release:
> path_release(&nd);
> @@ -760,7 +766,7 @@ asmlinkage long sys_fchown(unsigned int
> goto out_fput;
> dentry = file->f_path.dentry;
> audit_inode(NULL, dentry);
> - error = chown_common(dentry, file->f_path.mnt, user, group);
> + error = chown_common(dentry, file->f_path.mnt, user, group, file);
> mnt_drop_write(file->f_vfsmnt);
> out_fput:
> fput(file);
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -362,6 +362,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;
> };
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists