On Fri, Jul 15, 2005 at 05:45:58AM +0200, Jesper Juhl wrote:
> On 7/16/05, Nicholas Hans Simmonds <[email protected]> wrote:
>
> While I'm not qualified to comment on the implementation I do have a
> few small codingstyle comments :-)
>
>
> > diff --git a/fs/read_write.c b/fs/read_write.c
> > --- a/fs/read_write.c
> > +++ b/fs/read_write.c
> > @@ -14,6 +14,7 @@
> > #include <linux/security.h>
> > #include <linux/module.h>
> > #include <linux/syscalls.h>
> > +#include <linux/xattr.h>
> >
> > #include <asm/uaccess.h>
> > #include <asm/unistd.h>
> > @@ -303,6 +304,16 @@ ssize_t vfs_write(struct file *file, con
> > else
> > ret = do_sync_write(file, buf, count, pos);
> > if (ret > 0) {
> > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> > + struct dentry *d = file->f_dentry;
> > + if(d->d_inode->i_op && d->d_inode->i_op->
>
> if (d->d_inode->i_op ...
>
> > + removexattr) {
> > + down(&d->d_inode->i_sem);
> > + d->d_inode->i_op->removexattr(d,
> > + XATTR_CAP_SET);
> > + up(&d->d_inode->i_sem);
> > + }
> > +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
> > fsnotify_modify(file->f_dentry);
> > current->wchar += ret;
> > }
> > diff --git a/include/linux/capability.h b/include/linux/capability.h
> > --- a/include/linux/capability.h
> > +++ b/include/linux/capability.h
> > @@ -39,7 +39,19 @@ typedef struct __user_cap_data_struct {
> > __u32 permitted;
> > __u32 inheritable;
> > } __user *cap_user_data_t;
> > -
> > +
> > +struct cap_xattr_data {
> > + __u32 version;
> > + __u32 mask_effective;
> > + __u32 effective;
> > + __u32 mask_permitted;
> > + __u32 permitted;
> > + __u32 mask_inheritable;
> > + __u32 inheritable;
> > +};
> > +
> > +#define XATTR_CAP_SET XATTR_SECURITY_PREFIX "cap_set"
> > +
> > #ifdef __KERNEL__
> >
> > #include <linux/spinlock.h>
> > diff --git a/security/Kconfig b/security/Kconfig
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -60,6 +60,13 @@ config SECURITY_CAPABILITIES
> > This enables the "default" Linux capabilities functionality.
> > If you are unsure how to answer this question, answer Y.
> >
> > +config SECURITY_FS_CAPABILITIES
> > + bool "Filesystem Capabilities (EXPERIMENTAL)"
> > + depends on SECURITY && EXPERIMENTAL
> > + help
> > + This permits a process' capabilities to be set by an extended
> > + attribute in the security namespace (security.cap_set).
> > +
> > config SECURITY_ROOTPLUG
> > tristate "Root Plug Support"
> > depends on USB && SECURITY
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -111,9 +111,15 @@ void cap_capset_set (struct task_struct
> >
> > int cap_bprm_set_security (struct linux_binprm *bprm)
> > {
> > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> > + ssize_t (*bprm_getxattr)(struct dentry *,const char *,void *,size_t);
> > + struct dentry *bprm_dentry;
> > + ssize_t ret;
> > + struct cap_xattr_data caps;
> > +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
> > +
> > /* Copied from fs/exec.c:prepare_binprm. */
> >
> > - /* We don't have VFS support for capabilities yet */
> > cap_clear (bprm->cap_inheritable);
> > cap_clear (bprm->cap_permitted);
> > cap_clear (bprm->cap_effective);
> > @@ -134,6 +140,44 @@ int cap_bprm_set_security (struct linux_
> > if (bprm->e_uid == 0)
> > cap_set_full (bprm->cap_effective);
> > }
> > +
> > +#ifdef CONFIG_SECURITY_FS_CAPABILITIES
> > + /* Locate any VFS capabilities: */
> > +
> > + bprm_dentry = bprm->file->f_dentry;
> > + if(!(bprm_dentry->d_inode->i_op &&
>
> if (!(bprm_dentry->d_inode->i_op ...
>
> > + bprm_dentry->d_inode->i_op->getxattr))
> > + return 0;
> > + bprm_getxattr = bprm_dentry->d_inode->i_op->getxattr;
> > +
> > + down(&bprm_dentry->d_inode->i_sem);
> > + ret = bprm_getxattr(bprm_dentry,XATTR_CAP_SET,&caps,sizeof(caps));
> > + up(&bprm_dentry->d_inode->i_sem);
> > + if(ret == sizeof(caps)) {
>
> if (ret == sizeof(caps)) {
>
> > + be32_to_cpus(&caps.version);
> > + be32_to_cpus(&caps.effective);
> > + be32_to_cpus(&caps.mask_effective);
> > + be32_to_cpus(&caps.permitted);
> > + be32_to_cpus(&caps.mask_permitted);
> > + be32_to_cpus(&caps.inheritable);
> > + be32_to_cpus(&caps.mask_inheritable);
> > + if(caps.version == _LINUX_CAPABILITY_VERSION) {
>
> if (caps.version ...
>
> > + cap_t(bprm->cap_effective) &= caps.mask_effective;
> > + cap_t(bprm->cap_effective) |= caps.effective;
> > +
> > + cap_t(bprm->cap_permitted) &= caps.mask_permitted;
> > + cap_t(bprm->cap_permitted) |= caps.permitted;
> > +
> > + cap_t(bprm->cap_inheritable) &= caps.mask_inheritable;
> > + cap_t(bprm->cap_inheritable) |= caps.inheritable;
> > + } else
> > + printk(KERN_WARNING "Warning: %s capability set has "
> > + "incorrect version %08X. Correct version "
> > + "is %08X.\n",bprm->filename,caps.version,
> > + _LINUX_CAPABILITY_VERSION);
> > + }
> > +#endif /* CONFIG_SECURITY_FS_CAPABILITIES */
> > +
> > return 0;
> > }
> >
>
> --
> Jesper Juhl <[email protected]>
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please http://www.expita.com/nomime.html
Quite frankly if these are the only problems with my style I'll be more
than happy. The following patch cleans things up.
diff --git a/fs/read_write.c b/fs/read_write.c
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -306,8 +306,9 @@ ssize_t vfs_write(struct file *file, con
if (ret > 0) {
#ifdef CONFIG_SECURITY_FS_CAPABILITIES
struct dentry *d = file->f_dentry;
- if(d->d_inode->i_op && d->d_inode->i_op->
- removexattr) {
+ if (d->d_inode->i_op
+ && d->d_inode->i_op->removexattr)
+ {
down(&d->d_inode->i_sem);
d->d_inode->i_op->removexattr(d,
XATTR_CAP_SET);
diff --git a/security/commoncap.c b/security/commoncap.c
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -145,15 +145,15 @@ int cap_bprm_set_security (struct linux_
/* Locate any VFS capabilities: */
bprm_dentry = bprm->file->f_dentry;
- if(!(bprm_dentry->d_inode->i_op &&
- bprm_dentry->d_inode->i_op->getxattr))
+ if (!(bprm_dentry->d_inode->i_op
+ && bprm_dentry->d_inode->i_op->getxattr))
return 0;
bprm_getxattr = bprm_dentry->d_inode->i_op->getxattr;
down(&bprm_dentry->d_inode->i_sem);
ret = bprm_getxattr(bprm_dentry,XATTR_CAP_SET,&caps,sizeof(caps));
up(&bprm_dentry->d_inode->i_sem);
- if(ret == sizeof(caps)) {
+ if (ret == sizeof(caps)) {
be32_to_cpus(&caps.version);
be32_to_cpus(&caps.effective);
be32_to_cpus(&caps.mask_effective);
@@ -161,7 +161,7 @@ int cap_bprm_set_security (struct linux_
be32_to_cpus(&caps.mask_permitted);
be32_to_cpus(&caps.inheritable);
be32_to_cpus(&caps.mask_inheritable);
- if(caps.version == _LINUX_CAPABILITY_VERSION) {
+ if (caps.version == _LINUX_CAPABILITY_VERSION) {
cap_t(bprm->cap_effective) &= caps.mask_effective;
cap_t(bprm->cap_effective) |= caps.effective;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |