Re: No extended attributes for sticky dire ctories? (and samba)

--Boundary-00=_AphSFJI1rZN4oku
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Gerard,

On Thursday 26 October 2006 17:08, Dave Kleikamp wrote:
> On Thu, 2006-10-26 at 01:14 +1000, Gerard Neil wrote:
> > Hello,
> >
> > I have some queries about permissions for extended attributes in the
> > user.* namespace on sticky directories.
> >
> > The documented behaviour (from attr(5)) is that "access to extended
> > user attributes is restricted  to the owner and to users with
> > appropriate capabilities for directories with the sticky bit set".
> >
> > I did some digging and I understand why write access to extended
> > attributes needs to be restricted on mode 1777 directories like /tmp.
> > Fair enough, there's a potential DoS otherwise.
> >
> > What I don't understand is the current behaviour under linux (I'm
> > looking at current stable tree 2.6.18.1). The vfs code in fs/xattr.c
> > prevents *all* read or write access to extended attributes for sticky
> > directories, for *all* users (including root).
>
> I agree that this looks wrong.

Indeed, yes. The xfs code has it right, and the patch is good. I'll fix the 
two typos in the comment above in the same go, and push the resulting patch 
(attached) upstream. Thanks for your help!

> The patch looks good to me.  Rather than fix jfs in a similar manner,
> just leave it alone, and I'll patch jfs to remove the redundant
> permission checking and rely on the vfs.

Ack.

Cheers,
Andreas

--Boundary-00=_AphSFJI1rZN4oku
Content-Type: text/x-diff;
  charset="iso-8859-1";
  name="user-xattr-sticky-fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="user-xattr-sticky-fix.diff"

From: Andreas Gruenbacher <[email protected]>
Subject: Fix user.* xattr permission check for sticky dirs

The user.* extended attributes are only allowed on regular files and
directories. Sticky directories further restrict write access to the
owner and privileged users. (See the attr(5) man page for an
explanation.)

The original check in ext2/ext3 when user.* xattrs were merged was too
restrictive, and when the xattr permission checks were moved into the
VFS, read access to user.* attributes on sticky directores ended up
being denied as well. Change the permission check to what was originally
intended.

Originally-from: Gerard Neil <[email protected]>
Signed-off-by: Andreas Gruenbacher <[email protected]>

Index: linux-2.6.19-rc4/fs/xattr.c
===================================================================
--- linux-2.6.19-rc4.orig/fs/xattr.c
+++ linux-2.6.19-rc4/fs/xattr.c
@@ -48,14 +48,21 @@ xattr_permission(struct inode *inode, co
 		return 0;
 
 	/*
-	 * The trusted.* namespace can only accessed by a privilegued user.
+	 * The trusted.* namespace can only be accessed by a privileged user.
 	 */
 	if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN))
 		return (capable(CAP_SYS_ADMIN) ? 0 : -EPERM);
 
+	/* In user.* namespace, only regular files and directories can have
+	 * extended attributes. For sticky directories, only the owner and
+	 * privileged user can write attributes.
+	 */
 	if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
-		if (!S_ISREG(inode->i_mode) &&
-		    (!S_ISDIR(inode->i_mode) || inode->i_mode & S_ISVTX))
+		if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
+			return -EPERM;
+		if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
+		    (mask & MAY_WRITE) && (current->fsuid != inode->i_uid) &&
+		    !capable(CAP_FOWNER))
 			return -EPERM;
 	}
 

--Boundary-00=_AphSFJI1rZN4oku
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
acl-devel mailing list
[email protected]
http://acl.bestbits.at/mailman/listinfo/acl-devel

--Boundary-00=_AphSFJI1rZN4oku--