diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c index f6e202e6..b94c6c78 100644 --- a/libcomposefs/lcfs-writer.c +++ b/libcomposefs/lcfs-writer.c @@ -1660,24 +1660,26 @@ int lcfs_node_unset_xattr(struct lcfs_node_s *node, const char *name) { ssize_t index = find_xattr(node, name); - if (index >= 0) { - struct lcfs_xattr_s *xattr = &node->xattrs[index]; - size_t value_len = xattr->value_len; - free(xattr->key); - free(xattr->value); - if (index != (ssize_t)node->n_xattrs - 1) - node->xattrs[index] = node->xattrs[node->n_xattrs - 1]; - node->n_xattrs--; - // Note 2*size - to account for worst case alignment - if (node->n_xattrs > 0) - node->xattr_size -= (2 * LCFS_INODE_XATTRMETA_SIZE - 1) + - strlen(name) + value_len; - else - node->xattr_size = 0; // If last xattr, remove the overhead too - assert(node->xattr_size >= 0); + if (index < 0) { + errno = ENODATA; + return -1; } - return -1; + struct lcfs_xattr_s *xattr = &node->xattrs[index]; + size_t value_len = xattr->value_len; + free(xattr->key); + free(xattr->value); + if (index != (ssize_t)node->n_xattrs - 1) + node->xattrs[index] = node->xattrs[node->n_xattrs - 1]; + node->n_xattrs--; + // Note 2*size - to account for worst case alignment + if (node->n_xattrs > 0) + node->xattr_size -= (2 * LCFS_INODE_XATTRMETA_SIZE - 1) + + strlen(name) + value_len; + else + node->xattr_size = 0; // If last xattr, remove the overhead too + assert(node->xattr_size >= 0); + return 0; } /* Set an extended attribute; If from_external_input is true then we @@ -1702,7 +1704,8 @@ int lcfs_node_set_xattr_internal(struct lcfs_node_s *node, const char *name, } // Remove any existing value - (void)lcfs_node_unset_xattr(node, name); + if (lcfs_node_unset_xattr(node, name) < 0 && errno != ENODATA) + return -1; // Double the xattr metadata size, subtracting 1 to account for worst case alignment. size_t entry_size = (2 * LCFS_INODE_XATTRMETA_SIZE) - 1 + namelen + value_len; diff --git a/tests/test-lcfs.c b/tests/test-lcfs.c index 527271d1..fc536556 100644 --- a/tests/test-lcfs.c +++ b/tests/test-lcfs.c @@ -3,6 +3,7 @@ #include "lcfs-writer.h" #include "lcfs-mount.h" +#include #include #include #include @@ -58,6 +59,49 @@ static void test_basic(void) assert(r == 0); } +static void test_xattr_addremove(void) +{ + cleanup_node struct lcfs_node_s *node = lcfs_node_new(); + lcfs_node_set_mode(node, S_IFDIR | 0755); + cleanup_node struct lcfs_node_s *child = lcfs_node_new(); + lcfs_node_set_mode(child, S_IFDIR | 0700); + int r = lcfs_node_unset_xattr(child, "user.foo"); + int errsv = errno; + assert(r == -1); + assert(errsv == ENODATA); + r = lcfs_node_set_xattr(child, "user.foo", "bar", 3); + assert(r == 0); + r = lcfs_node_unset_xattr(child, "user.foo"); + assert(r == 0); + r = lcfs_node_add_child(node, child, "somechild"); + assert(r == 0); + child = NULL; +} + +// Test that calling lcfs_node_set_xattr multiple times +// with the same key has last-one-wins semantics. +static void test_xattr_doubleadd(void) +{ + cleanup_node struct lcfs_node_s *node = lcfs_node_new(); + lcfs_node_set_mode(node, S_IFDIR | 0755); + cleanup_node struct lcfs_node_s *child = lcfs_node_new(); + lcfs_node_set_mode(child, S_IFDIR | 0700); + int r = lcfs_node_set_xattr(child, "user.foo", "bar", 3); + assert(r == 0); + // Should successfully silently overwrite. + r = lcfs_node_set_xattr(child, "user.foo", "baz", 3); + assert(r == 0); + + size_t found_len; + const char *found_value = lcfs_node_get_xattr(child, "user.foo", &found_len); + assert(found_value); + assert(found_len == 3); + assert(memcmp(found_value, "baz", found_len) == 0); + r = lcfs_node_add_child(node, child, "somechild"); + assert(r == 0); + child = NULL; +} + static void test_add_uninitialized_child(void) { cleanup_node struct lcfs_node_s *node = lcfs_node_new(); @@ -99,4 +143,6 @@ int main(int argc, char **argv) test_basic(); test_no_verity(); test_add_uninitialized_child(); + test_xattr_addremove(); + test_xattr_doubleadd(); }