NetBSD Problem Report #48374

From jarmo.jaakkola@roskakori.fi  Sat Nov  9 13:10:19 2013
Return-Path: <jarmo.jaakkola@roskakori.fi>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 244DFA618A
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  9 Nov 2013 13:10:19 +0000 (UTC)
Message-Id: <20131109131013.54969530C@roskakori.fi>
Date: Sat,  9 Nov 2013 15:10:13 +0200 (EET)
From: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
Reply-To: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
To: gnats-bugs@gnats.NetBSD.org
Subject: Linux API in kern/vfs_xattr.c does not work
X-Send-Pr-Version: 3.95

>Number:         48374
>Notify-List:    bsiegert@NetBSD.org
>Category:       kern
>Synopsis:       Namespace is not removed from attrname in Linux style xattr API
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 09 13:15:00 +0000 2013
>Last-Modified:  Sat Nov 09 16:08:15 +0000 2013
>Originator:     Jarmo Jaakkola
>Release:        NetBSD 6.1.1
>Organization:
>Environment:
System: NetBSD kotoisa.roskakori.fi 6.1.1 NetBSD 6.1.1 (KOTOISA) #3: Sat Nov 9 12:55:00 EET 2013 jammuli@kotoisa.roskakori.fi:/usr/src/sys/arch/i386/compile/KOTOISA i386
Architecture: i386
Machine: i386
>Description:
The Linux compatible API for extended attributes in kern/vfs_xattr.c
figures out the numerical namespace id from the textual prefix provided in
the name parameter of the Linux style call.  However, this prefix is still
present in attrname when the call to the actual implementation is made.

This results in a call asking to process attribute "ns.attr" instead of
"attr", which of course does not work.
>How-To-Repeat:
There surely is a more minimalistic test case for this, along the lines of
"use setxattr() to set attribute 'foo.bar'", but this is how I came by this
problem.

    1. Install net/samba.  Detects EA support, compiles fine.
    2. Enable "store dos attributes" in smb.conf for some share.
    3. Enable user.DOSATTRIB in the filesystem housing said share (I used
       128 bytes per value)
    4. Verify with getextattr(1), setextattr(1), rmextattr(1) that
       the attribute is indeed usable
    5. Use smbclient to set e.g. hidden attribute on a file in the share:
        smb> setmode some_file +h
    6. List the files on the share:
        smb> ls
       Notice that some_file does not have +H set.
>Fix:
Modify relevant functions to skip the namespace prefix in attrname when
passing the value forward.  Now Samba can store and retieve the attributes.

The following patch modifies all of the functions that used xattr_native()
and this worked for my use case.  There were however some functions 
(e.g. sys_flistxattr) which pull the namespace ID out of thin air.
Someone who actually knows about this stuff should see if they need to
be modified too.


--- vfs_xattr.c.orig	2013-11-09 13:45:55.000000000 +0200
+++ vfs_xattr.c	2013-11-09 13:45:55.000000000 +0200
@@ -769,19 +769,26 @@
  * Linux-compatible <sys/xattr.h> API for file system extended attributes
  *****************************************************************************/

-#define MATCH_NS(ns, key) (strncmp(ns, key, sizeof(ns) - 1) == 0)
+#define MATCH_NS(ns, key, ns_len) (*ns_len = sizeof(ns) - 1, \
+	strncmp(ns, key, *ns_len) == 0)
 static int
-xattr_native(const char *key) {
-	if (MATCH_NS("system.", key))
+xattr_native(const char *key, size_t *ns_len) {
+	if (MATCH_NS("system.", key, ns_len))
 		return EXTATTR_NAMESPACE_SYSTEM;
-	else if (MATCH_NS("user.", key))
+	else if (MATCH_NS("user.", key, ns_len))
 		return EXTATTR_NAMESPACE_USER;
-	else if (MATCH_NS("security.", key))
+	else if (MATCH_NS("security.", key, ns_len))
 		return EXTATTR_NAMESPACE_SYSTEM;
-	else if (MATCH_NS("trusted.", key))
+	else if (MATCH_NS("trusted.", key, ns_len))
 		return EXTATTR_NAMESPACE_SYSTEM;
-	else 
+	else {
+		*ns_len = 0;
+		for (const char *i = key; *i; ++i)
+			if ('.' == *i)
+				*ns_len = (i + 1) - key;
+				
 		return EXTATTR_NAMESPACE_USER;
+	}

 }
 #undef MATCH_NS
@@ -800,6 +807,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	register_t attrlen;
 	int error;
@@ -814,10 +822,10 @@
 	if (error)
 		goto out;

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_set_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l, 
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size), l, 
 	    &attrlen, SCARG(uap, flags));

 	vrele(vp);
@@ -838,6 +846,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	register_t attrlen;
 	int error;
@@ -852,10 +861,10 @@
 	if (error)
 		goto out;

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_set_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l,
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size), l,
 	    &attrlen, SCARG(uap, flags));

 	vrele(vp);
@@ -877,6 +886,7 @@
 	struct file *fp;
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	register_t attrlen;
 	int error;
@@ -891,10 +901,10 @@
 		goto out;
 	vp = (struct vnode *) fp->f_data;

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_set_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l,
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size), l,
 	    &attrlen, SCARG(uap, flags));

 	fd_putfile(SCARG(uap, fd));
@@ -914,6 +924,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -927,10 +938,11 @@
 	if (error)
 		return (error);

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_get_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l, retval);
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size),
+	    l, retval);

 	vrele(vp);
 	return (XATTR_ERRNO(error));
@@ -947,6 +959,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -960,10 +973,11 @@
 	if (error)
 		return (error);

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_get_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l, retval);
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size),
+	    l, retval);

 	vrele(vp);
 	return (XATTR_ERRNO(error));
@@ -981,6 +995,7 @@
 	struct file *fp;
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -994,10 +1009,11 @@
 		return (error);
 	vp = (struct vnode *) fp->f_data;

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

 	error = extattr_get_vp(vp, namespace,
-	    attrname, SCARG(uap, value), SCARG(uap, size), l, retval);
+	    attrname + namespace_len, SCARG(uap, value), SCARG(uap, size),
+	    l, retval);

 	fd_putfile(SCARG(uap, fd));
 	return (XATTR_ERRNO(error));
@@ -1170,6 +1186,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -1183,9 +1200,9 @@
 	if (error)
 		return (error);

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

-	error = extattr_delete_vp(vp, namespace, attrname, l);
+	error = extattr_delete_vp(vp, namespace, attrname + namespace_len, l);

 	vrele(vp);
 	return (XATTR_ERRNO(error));
@@ -1200,6 +1217,7 @@
 	} */
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -1213,9 +1231,9 @@
 	if (error)
 		return (error);

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

-	error = extattr_delete_vp(vp, namespace, attrname, l);
+	error = extattr_delete_vp(vp, namespace, attrname + namespace_len, l);

 	vrele(vp);
 	return (XATTR_ERRNO(error));
@@ -1231,6 +1249,7 @@
 	struct file *fp;
 	struct vnode *vp;
 	char attrname[XATTR_NAME_MAX];
+	size_t namespace_len;
 	int namespace;
 	int error;

@@ -1244,9 +1263,9 @@
 		return (error);
 	vp = (struct vnode *) fp->f_data;

-	namespace = xattr_native(attrname);
+	namespace = xattr_native(attrname, &namespace_len);

-	error = extattr_delete_vp(vp, namespace, attrname, l);
+	error = extattr_delete_vp(vp, namespace, attrname + namespace_len, l);

 	fd_putfile(SCARG(uap, fd));
 	return (XATTR_ERRNO(error));

>Release-Note:

>Audit-Trail:
From: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/48374: Linux API in kern/vfs_xattr.c does not work
Date: Sat, 9 Nov 2013 15:28:14 +0200

 Heads up!

 There's a "break" missing here:
     +		*ns_len = 0;
     +		for (const char *i = key; *i; ++i)
     +			if ('.' == *i)
     +				*ns_len = (i + 1) - key;

 Assuming we even want to consider any unknown (and possibly missing!)
 prefix as valid and equivalent to "user", this should be:
     +		*ns_len = 0;
     +		for (const char *i = key; *i; ++i)
     +			if ('.' == *i) {
     +				*ns_len = (i + 1) - key;
     +				break;
     +			}

 -- 
 Jarmo Jaakkola

>Unformatted:

Home	PR Database Search