|
| From: | dai.shixin |
| Subject: | [PATCH] setfacl is not atomic and corresponding repair solution |
| Date: | Tue, 13 Aug 2024 15:40:26 +0800 (CST) |
From: dai.shixin@zte.com.cn
Hi,
We have found a issue about setfacl, the detail is as followed,
1. Issue description: we update ACL rules for a same directory when multiple containers launched, but the ACL rules may be lost occasionally. However, no error is reported by setfacl.For example, after running command `setfacl -R -m u:1234:rwx /mnt`, the command is executed successfully, but the /mnt directory does not have the ACL rule for user 1234.
2. Cause analysis: It is found that the read/write process of the ACL rule set by setfacl is not atomic. For example, setfacl instance A will overwrites the rule saved by instance B.
setfacl instance A： get_acl() set_acl()
setfacl instance B： get_acl() set_acl()
3. Recommended solution: Using the flock mechanism provided by kernel to guarantee the read/write process is atomic.The approximate solution is described as follows:
int
do_set(
const char *path_p,
const struct stat *st,
int walk_flags,
void *arg)
{
........
+ /* skip setting if the rule is redundant */
+ if (do_set_need_skip())：
return；
+ /* add lock to guarantee the atomic read-write process */
+ flock(path_p);
RETRIEVE_ACL(cmd->c_type); get acl rules
.............
if (acl_set_file(path_p, ACL_TYPE_ACCESS, acl) != 0) { set acl rules
+ unlock();
.......
}
Signed-off-by: dai.shixin@zte.com.cn
Reviewed-by: ran.xiaokai@zte.com.cn
Reviewed-by: zhang.wenya1@zte.com.cn
Reviewed-by: yang.yang29@zte.com.cn
Reviewed-by: si.hao@zte.com.cn
---
tools/do_set.c | 264 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 264 insertions(+)
diff --git a/tools/do_set.c b/tools/do_set.c
index 696689c..d083f8b 100644
--- a/tools/do_set.c
+++ b/tools/do_set.c
@@ -38,6 +38,7 @@
#include "do_set.h"
#include "parse.h"
#include "walk_tree.h"
+#include <sys/file.h>
static acl_entry_t
@@ -251,6 +252,223 @@ remove_extended_entries(
goto fail; \
} while(0)
+int
+do_set_need_skip(
+ const char *path_p,
+ const struct stat *st,
+ int walk_flags,
+ void *arg)
+{
+ struct do_set_args *args = arg;
+ acl_t old_acl = NULL, old_default_acl = NULL;
+ acl_t acl = NULL, default_acl = NULL;
+ acl_t *xacl, *old_xacl;
+ acl_entry_t ent;
+ cmd_t cmd;
+ int which_entry;
+ int skip = 0, error;
+ char *acl_text;
+ int acl_modified = 0, default_acl_modified = 0;
+ int acl_mask_provided = 0, default_acl_mask_provided = 0;
+
+ /* Execute the commands in seq (read ACLs on demand) */
+ error = seq_get_cmd(args->seq, SEQ_FIRST_CMD, &cmd);
+ if (error == 0)
+ return 0;
+
+ while (error == 1) {
+ mode_t perm = cmd->c_perm;
+
+ if (cmd->c_type == ACL_TYPE_ACCESS) {
+ xacl = &acl;
+ old_xacl = &old_acl;
+ acl_modified = 1;
+ if (cmd->c_tag == ACL_MASK)
+ acl_mask_provided = 1;
+ } else {
+ xacl = &default_acl;
+ old_xacl = &old_default_acl;
+ default_acl_modified = 1;
+ if (cmd->c_tag == ACL_MASK)
+ default_acl_mask_provided = 1;
+ }
+
+ RETRIEVE_ACL(cmd->c_type);
+
+ /* Check for `X', and replace with `x' as appropriate. */
+ if (perm & CMD_PERM_COND_EXECUTE) {
+ perm &= ~CMD_PERM_COND_EXECUTE;
+ if (S_ISDIR(st->st_mode) || has_execute_perms(*xacl))
+ perm |= CMD_PERM_EXECUTE;
+ }
+
+ switch(cmd->c_cmd) {
+ case CMD_ENTRY_REPLACE:
+ ent = find_entry(*xacl, cmd->c_tag, cmd->c_id);
+ if (!ent) {
+ if (acl_create_entry(xacl, &ent) != 0)
+ goto fail;
+ acl_set_tag_type(ent, cmd->c_tag);
+ if (cmd->c_id != ACL_UNDEFINED_ID)
+ acl_set_qualifier(ent,
+ &cmd->c_id);
+ }
+ set_perm(ent, perm);
+ break;
+
+ case CMD_REMOVE_ENTRY:
+ ent = find_entry(*xacl, cmd->c_tag, cmd->c_id);
+ if (ent)
+ acl_delete_entry(*xacl, ent);
+ else
+ /* ignore */;
+ break;
+
+ case CMD_REMOVE_EXTENDED_ACL:
+ remove_extended_entries(acl);
+ break;
+
+ case CMD_REMOVE_ACL:
+ acl_free(*xacl);
+ *xacl = acl_init(5);
+ if (!*xacl)
+ goto fail;
+ break;
+
+ default:
+ errno = EINVAL;
+ goto fail;
+ }
+
+ error = seq_get_cmd(args->seq, SEQ_NEXT_CMD, &cmd);
+ }
+
+ if (error < 0)
+ goto fail;
+
+ /* Try to fill in missing entries */
+ if (default_acl && acl_entries(default_acl) != 0) {
+ xacl = &acl;
+ old_xacl = &old_acl;
+
+ if (!find_entry(default_acl, ACL_USER_OBJ, ACL_UNDEFINED_ID)) {
+ if (!acl)
+ RETRIEVE_ACL(ACL_TYPE_ACCESS);
+ clone_entry(acl, ACL_USER_OBJ,
+ &default_acl, ACL_USER_OBJ);
+ }
+ if (!find_entry(default_acl, ACL_GROUP_OBJ, ACL_UNDEFINED_ID)) {
+ if (!acl)
+ RETRIEVE_ACL(ACL_TYPE_ACCESS);
+ clone_entry(acl, ACL_GROUP_OBJ,
+ &default_acl, ACL_GROUP_OBJ);
+ }
+ if (!find_entry(default_acl, ACL_OTHER, ACL_UNDEFINED_ID)) {
+ if (!acl)
+ RETRIEVE_ACL(ACL_TYPE_ACCESS);
+ clone_entry(acl, ACL_OTHER,
+ &default_acl, ACL_OTHER);
+ }
+ }
+
+ /* update mask entries and check if ACLs are valid */
+ if (acl && acl_modified) {
+ if (acl_equiv_mode(acl, NULL) != 0) {
+ if (!acl_mask_provided &&
+ !find_entry(acl, ACL_MASK, ACL_UNDEFINED_ID))
+ clone_entry(acl, ACL_GROUP_OBJ,
+ &acl, ACL_MASK);
+ if (opt_recalculate != -1 &&
+ (!acl_mask_provided || opt_recalculate == 1))
+ acl_calc_mask(&acl);
+ }
+
+ error = acl_check(acl, &which_entry);
+ if (error < 0)
+ goto fail;
+ if (error > 0) {
+ acl_text = acl_to_any_text(acl, NULL, ',', 0);
+ fprintf(stderr, _("%s: %s: Malformed access ACL "
+ "`%s': %s at entry %d\n"), progname, path_p,
+ acl_text, acl_error(error), which_entry+1);
+ acl_free(acl_text);
+ goto cleanup;
+ }
+ }
+
+ if (default_acl && acl_entries(default_acl) != 0 &&
+ default_acl_modified) {
+ if (acl_equiv_mode(default_acl, NULL) != 0) {
+ if (!default_acl_mask_provided &&
+ !find_entry(default_acl,ACL_MASK,ACL_UNDEFINED_ID))
+ clone_entry(default_acl, ACL_GROUP_OBJ,
+ &default_acl, ACL_MASK);
+ if (opt_recalculate != -1 &&
+ (!default_acl_mask_provided ||
+ opt_recalculate == 1))
+ acl_calc_mask(&default_acl);
+ }
+
+ error = acl_check(default_acl, &which_entry);
+ if (error < 0)
+ goto fail;
+ if (error > 0) {
+ acl_text = acl_to_any_text(default_acl, NULL, ',', 0);
+ fprintf(stderr, _("%s: %s: Malformed default ACL "
+ "`%s': %s at entry %d\n"),
+ progname, path_p, acl_text,
+ acl_error(error), which_entry+1);
+ acl_free(acl_text);
+ goto cleanup;
+ }
+ }
+
+ /* Only directores can have default ACLs */
+ if (default_acl && !S_ISDIR(st->st_mode) && (walk_flags & WALK_TREE_RECURSIVE)) {
+ /* In recursive mode, ignore default ACLs for files */
+ acl_free(default_acl);
+ default_acl = NULL;
+ }
+ /* check which ACLs have changed */
+ if (acl && old_acl && acl_cmp(old_acl, acl) == 0) {
+ acl_free(acl);
+ acl = NULL;
+ }
+ if ((default_acl && old_default_acl &&
+ acl_cmp(old_default_acl, default_acl) == 0)) {
+ acl_free(default_acl);
+ default_acl = NULL;
+ }
+
+ /* update the file system */
+ if (opt_test) {
+ print_test(stdout, path_p, st,
+ acl, default_acl);
+ skip = 1;
+ goto cleanup;
+ }
+
+ /* provided that the rules are updated, skip the later procudure */
+ if (acl || default_acl)
+ skip = 0;
+ else
+ skip = 1;
+
+cleanup:
+ if (acl)
+ acl_free(acl);
+ if (old_acl)
+ acl_free(old_acl);
+ if (default_acl)
+ acl_free(default_acl);
+ if (old_default_acl)
+ acl_free(old_default_acl);
+ return skip;
+
+fail:
+ goto cleanup;
+}
+
int
do_set(
const char *path_p,
@@ -269,6 +487,8 @@ do_set(
char *acl_text;
int acl_modified = 0, default_acl_modified = 0;
int acl_mask_provided = 0, default_acl_mask_provided = 0;
+ int fd = -1, ret = -1, tried = 1, locked = 0;
+ const int MAX_TRIED_TIMES = 3;
if (walk_flags & WALK_TREE_FAILED) {
fprintf(stderr, "%s: %s: %s\n", progname, path_p, strerror(errno));
@@ -285,10 +505,50 @@ do_set(
!(walk_flags & (WALK_TREE_TOPLEVEL | WALK_TREE_LOGICAL))))
return 0;
+ /* skip setting if the rule is redundant */
+ if (do_set_need_skip(path_p, st, walk_flags, arg) == 1)
+ return 0;
+
/* Execute the commands in seq (read ACLs on demand) */
error = seq_get_cmd(args->seq, SEQ_FIRST_CMD, &cmd);
if (error == 0)
return 0;
+
+ /* add lock to guarantee the atomic read-write process */
+ fd = open(path_p, O_RDONLY);
+ if (fd == -1) {
+ printf("setfacl: open %s failed\n", path_p);
+ return 1;
+ }
+ /* try MAX_TRIED_TIMES times */
+ while (ret < 0)
+ {
+ ret = flock(fd, LOCK_EX | LOCK_NB);
+ if (ret == -1)
+ {
+ if (errno == EWOULDBLOCK)
+ {
+ printf("setfacl: %s is locked, retry\n", path_p);
+ tried++;
+ usleep(10000);
+ if (tried > MAX_TRIED_TIMES)
+ {
+ printf("setfacl: %s is locked by others for too long\n", path_p);
+ /* once failed to require lock, then report error */
+ errors ++;
+ goto cleanup;
+ }
+ }
+ else
+ {
+ printf("setfacl: flock(%s) failed, errno = %d, reason is %s\n", path_p, errno, strerror(errno));
+ errors ++;
+ goto cleanup;
+ }
+ }
+ }
+ locked = 1;
+
while (error == 1) {
mode_t perm = cmd->c_perm;
@@ -511,6 +771,10 @@ do_set(
error = 0;
cleanup:
+ if(locked)
+ flock(fd, LOCK_UN);
+ if(fd != -1)
+ close(fd);
if (acl)
acl_free(acl);
if (old_acl)
--
2.27.0
| [Prev in Thread] | Current Thread | [Next in Thread] |