Re: [systemd-devel] [PATCHv4] tmpfiles, man: Add xattr support to tmpfiles

[Lennart Poettering]

On Thu, 08.08.13 13:54, Maciej Wereski (m.were...@partner.samsung.com) wrote:

 +value = strreplace(tmp, \\\, \);
 +if (!value)
 +return log_oom();

This looks like a job for cunescape() or so?

 +n = strlen(value);
 +if (i-type == CREATE_SYMLINK) {
 +if (lsetxattr(path, name, value, n+1, 0)  0) {
 +log_error(Setting extended attribute %s=%s 
 on symlink %s failed: %m, name, value, path);
 +return -errno;
 +}
 +}
 +else if (setxattr(path, name, value, n+1, 0)  0) {
 +log_error(Setting extended attribute %s=%s on %s 
 failed: %m, name, value, path);
 +return -errno;
 +}
 +}
 +return 0;
 +#else
 +(void)i;
 +(void)path;
 +log_error(Setting extended attributes requested, but tmpfiles was 
 compiled without XATTR support!);
 +return -ENOTSUP;

If I follow the control flow correctly then this message would be
generated for *all* entries if XATTR support is not compiled in,
regardless whether xattrs shall actually be set... This part should come
after the early-return check whether the xattr list is actually empty...

 +xattr = new0(char, strlen(i-argument)+1);
 +if (!xattr)
 +return log_oom();
 +
 +tmp = strv_split(i-argument, WHITESPACE);
 +if (!tmp)
 +return log_oom();
 +
 +for (n = 0; n  strv_length(tmp); ++n) {

strv_length() is relatively expensive (O(n)), please cache the result.


 +len = strlen(tmp[n]);
 +strncpy(xattr, tmp[n], len+1);
 +p = strchr(xattr, '=');
 +if (!p) {
 +log_error(%s: Attribute has incorrect format., 
 i-path);
 +return -EBADMSG;
 +}
 +if (p[1] == '\') {
 +while (true) {
 +if (!p)
 +p = tmp[n];
 +else
 +p += 2;
 +p = strchr(p, '\');
 +if (p  xattr[p-xattr-1] != '\\')
 +break;
 +p = NULL;
 +++n;
 +if (n == strv_length(tmp))
 +break;
 +len += strlen(tmp[n]) + 1;
 +strncat(xattr,  , 1);
 +strncat(xattr, tmp[n], len);
 +}
 +}
 +strstrip(xattr);

Hmm, this also is a job for split_pair and cunescape, no? Or am I confused?

Otherwise looks fine!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv4] tmpfiles, man: Add xattr support to tmpfiles

[Maciej Wereski]

This patch makes it possible to set extended attributes on files created
by tmpfiles. This can be especially used to set SMACK security labels on
volatile files and directories.

It is done by adding new line of type t. Such line should contain
attributes in Argument field, using following format:

name=value

All other fields are ignored.

If value contains spaces, then it must be surrounded by quotation marks.
User can also put quotation mark in value by escaping it with backslash.

Example:
D /var/run/cups - - - -
t /var/run/cups - - - - security.SMACK64=printing
---
v4:
* grammar fix in man
* style fix

v3:
* may be used instead of should be used in manpage
* use strv_isempty() instead of != NULL
* rework item_set_xattrs() with split_pair()
* remove copy_item_contents()
* use hashmap_replace() instead of removed copy_item_contents()
* use strv_extend() instead of strv_append()
* cleanup
---
 man/tmpfiles.d.xml  |  26 ++-
 src/tmpfiles/tmpfiles.c | 203 +---
 2 files changed, 213 insertions(+), 16 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 6a2193d..41226c3 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -229,6 +229,21 @@ L/tmp/foobar ----   
/dev/null/programlisting
 place of normal path
 names./para/listitem
 /varlistentry
+
+varlistentry
+termvarnamet/varname/term
+listitemparaSet extended
+attributes on item. It may be
+used in conjunction with other
+types (only d, D, f, F, L, p, c, b, z
+makes sense). If used as a standalone
+line, then commandsystemd-tmpfiles
+/command will try to set extended
+attributes on specified path.
+This can be especially used to set
+SMACK labels.
+/para/listitem
+/varlistentry
 /variablelist
 /refsect2
 
@@ -242,7 +257,7 @@ L/tmp/foobar ----   
/dev/null/programlisting
 objects. For z, Z lines if omitted or when set
 to - the file access mode will not be
 modified. This parameter is ignored for x, r,
-R, L lines./para
+R, L, t lines./para
 /refsect2
 
 refsect2
@@ -254,7 +269,7 @@ L/tmp/foobar ----   
/dev/null/programlisting
 omitted or when set to - the default 0 (root)
 is used. For z, Z lines when omitted or when set to -
 the file ownership will not be modified.
-These parameters are ignored for x, r, R, L 
lines./para
+These parameters are ignored for x, r, R, L, t 
lines./para
 /refsect2
 
 refsect2
@@ -307,8 +322,10 @@ L/tmp/foobar ----   
/dev/null/programlisting
 minor formatted as integers, separated by :,
 e.g. 1:3. For f, F, w may be used to specify
 a short string that is written to the file,
-suffixed by a newline. Ignored for all other
+suffixed by a newline. Fot t determines extended
+attributes to be set. Ignored for all other
 lines./para
+
 /refsect2
 
 /refsect1
@@ -320,7 +337,8 @@ L/tmp/foobar ----   
/dev/null/programlisting
 paracommandscreen/command needs two directories 
created at boot with specific modes and ownership./para
 
 programlistingd /var/run/screens  1777 root root 10d
-d /var/run/uscreens 0755 root root 10d12h/programlisting
+d /var/run/uscreens 0755 root root 10d12h
+t /var/run/screen - - - - user.name=John Koval 
security.SMACK64=screen/programlisting
 /example
 /refsect1
 
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 5eca82a..a6594b1 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -39,6 +39,9 @@
 #include glob.h
 #include fnmatch.h
 #include sys/capability.h
+#ifdef HAVE_XATTR
+#include attr/xattr.h
+#endif
 
 #include log.h
 #include util.h
@@ -75,7 +78,10 @@ typedef enum ItemType {
 REMOVE_PATH = 'r',
 RECURSIVE_REMOVE_PATH = 'R',
 RELABEL_PATH =