Quoting Stephen Smalley ([email protected]):
> On Mon, 2007-02-19 at 11:01 -0600, Serge E. Hallyn wrote:
> > From: Serge E. Hallyn <[email protected]>
> > Subject: [PATCH -mm] file caps: make on-disk capabilities future-proof
> >
> > Stephen Smalley has pointed out that the current file capabilities
> > will eventually pose a problem.
> >
> > As the capability set changes and distributions start tagging
> > binaries with capabilities, we would like for running an older
> > kernel to not necessarily make those binaries unusable. To
> > that end,
> >
> > 1. If capabilities are specified which we don't know
> > about, just ignore them, do not return -EPERM as we
> > were doing before.
>
> I didn't advocate that change - it is a separate issue from allowing the
> capability bitmaps to grow in size in a backward compatible manner. In
> the one case, you have a binary that needs a capability that is unknown
> to the kernel, so running it could lead to unexpected failure. In the
> other case, you simply have a binary labeled by newer userspace with a
> newer on-disk representation that supports larger bitmaps, but the
> binary might only have capabilities set that are known to the kernel.
So do you think we should fail with -EINVAL in the first case?
> > 2. Specify a size with the on-disk capability implementation.
> > In this implementation the size is the number of __u32's
> > used for each of (eff,perm,inh). For now, sz is 1.
> > When we move to 64-bit capabilities, it becomes 2.
>
> You could alternatively split them into separate xattrs, e.g.
> security.cap.eff, security.cap.perm, security.cap.inh, and determine the
> bitmap size from the xattr length rather than a separate field.
Clean, but slower... Not sure which way to go on that
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > index be86acb..dc8bf4f 100644
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
>
> > @@ -148,50 +145,65 @@ static int set_file_caps(struct linux_bi
> > {
> > struct dentry *dentry;
> > ssize_t rc;
> > - struct vfs_cap_data_disk dcaps;
> > + struct vfs_cap_data_disk *dcaps;
> > struct vfs_cap_data caps;
> > struct inode *inode;
> > - int err;
> >
> > if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)
> > return 0;
> >
> > dentry = dget(bprm->file->f_dentry);
> > inode = dentry->d_inode;
> > - if (!inode->i_op || !inode->i_op->getxattr) {
> > - dput(dentry);
> > - return 0;
> > + rc = 0;
> > + if (!inode->i_op || !inode->i_op->getxattr)
> > + goto out;
> > +
> > + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
> > + if (rc == -ENODATA) {
> > + rc = 0;
> > + goto out;
> > + }
>
> I'd allocate an initial buffer with an expected size and try first to
> avoid always having to make the two ->getxattr calls in the common case.
I started to do that but decided that's just muck up the rfc. Will put
it into a final version.
> > + if (rc < 0)
> > + goto out;
> > + if (rc < sizeof(struct vfs_cap_data_disk)) {
>
> You could make this a bit stricter, as you know that it will have at
> least three additional u32 values beyond the header.
true.
> > + rc = -EINVAL;
> > + goto out;
> > + }
> > +
> > + dcaps = kmalloc(rc, GFP_KERNEL);
> > + if (!dcaps) {
> > + rc = -ENOMEM;
> > + goto out;
> > + }
> > + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
> > + XATTR_CAPS_SZ);
>
> I'm confused - you just asked for the actual length of the xattr and
> allocated a buffer for it, and then don't use the length in this second
Huh, I *did* send in rc, not sure what happened to that. git mis-usage
maybe.
> call to ->getxattr. And since you said you were organizing it as
> eff[0..sz-1],perm[0..sz-1],inh[0..sz-1], you do need to read the entire
> thing to get all three values even if you only use the lower 32 bits of
> each. Or if you change the organization to avoid the need to read the
> entire thing, you don't need the first getxattr call at all, and you
Yes I had first organized it as eff[0],perm[0],inh[0], eff[1], etc...
But after I changed that I did put rc back in for the length... I
thought.
> need to change how cap_from_disk extracts the values.
thanks Stephen,
-serge
-
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]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
