Quoting Andrew Morgan ([EMAIL PROTECTED]):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Serge,
>
> I guess I'm not sure what to do about this.
>
> In the caller there is an explicit check for negative rc in which case
> the modifed function is not called.
Yeah, and it's only used to compare to two constants anyway. (I was
thinking it was used more directly to control # bytes copied, but it
isn't.) It does seem to have the potential of confusing future coders,
so from that point of view it might be safest to have
if (rc < 0)
goto out;
unsigned size = (unsigned)rc;
rc = cap_from_disk(v1caps, bprm, size);
But it sure looks like I'm being pedantic so please feel free to ignore
me.
thanks,
-serge
> The argument really is an unsigned quantity and I felt this change was
> an improvement/fix.
>
> Can you suggest a change that would satisfy you here?
>
> Thanks
>
> Andrew
>
> Serge E. Hallyn wrote:
>
> > Other than the one comment below,
>
> > Acked-by: Serge Hallyn <[EMAIL PROTECTED]>
>
>
> >>
> - -static inline int cap_from_disk(__le32 *caps, struct linux_binprm *bprm,
> - - int size)
> +static inline int cap_from_disk(struct vfs_cap_data *caps,
> + struct linux_binprm *bprm, unsigned size)
>
> > Note that you switched this to unsigned, but the caller is still sending
> > in an int (rc).
>
> [..]
>
> @@ -219,7 +245,7 @@ static int get_file_caps(struct linux_binprm *bprm)
> {
> struct dentry *dentry;
> int rc = 0;
> - - __le32 v1caps[XATTR_CAPS_SZ];
> + struct vfs_cap_data vcaps;
> struct inode *inode;
> >>
> if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
> @@ -232,8 +258,8 @@ static int get_file_caps(struct linux_binprm *bprm)
> if (!inode->i_op || !inode->i_op->getxattr)
> goto out;
> >>
> - - rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &v1caps,
> - - XATTR_CAPS_SZ);
> + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &vcaps,
> + XATTR_CAPS_SZ);
> if (rc == -ENODATA || rc == -EOPNOTSUPP) {
> /* no data, that's ok */
> rc = 0;
> @@ -242,7 +268,7 @@ static int get_file_caps(struct linux_binprm *bprm)
> if (rc < 0)
> goto out;
> >>
> - - rc = cap_from_disk(v1caps, bprm, rc);
> + rc = cap_from_disk(&vcaps, bprm, rc);
> if (rc)
> printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n",
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (Darwin)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFHNTXnmwytjiwfWMwRAl+SAKCWzzeTd/5/gRA3wqE+cb9yfPS9cwCfVjC0
> w4D0isaFXnOCW77WcG+1d7o=
> =kRqk
> -----END PGP SIGNATURE-----
-
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
