From bdfa812d79c3b6e41772dac6d557e1ae6a3da4d8 Mon Sep 17 00:00:00 2001 From: CismonX Date: Thu, 27 Mar 2025 12:36:27 +0800 Subject: [PATCH] backend: respect the BOOKMARK_DELETE_DIR flag Following commit 2e3685f217b5167b4f7b85d1a1a96ccacfb736d4, make sure all backends check this flag and return correct error codes. Normally this is not mandatory, since the kernel looks up the directory entry to be removed, and fails if the system call is inappropriate (e.g., calling rmdir() on a regular file). This happens before FUSE_UNLINK or FUSE_RMDIR is sent to the server. However, when not in exclusive mode, there is a short window that TOCTOU problem may occur, which may lead to undesired behavior (e.g., deletion of a non-empty directory) or even the corruption of bookmark storage if not properly checked. Also explain this flag in the user manual. --- doc/bookmarkfs.texi | 7 +++++++ src/backend_chromium.c | 19 ++++++++++++------- src/backend_firefox.c | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/doc/bookmarkfs.texi b/doc/bookmarkfs.texi index e13627e..404fa3a 100644 --- a/doc/bookmarkfs.texi +++ b/doc/bookmarkfs.texi @@ -2808,6 +2808,13 @@ depending on the value of @code{flags}. A bit array of the following flags: @table @code +@item BOOKMARKFS_BOOKMARK_DELETE_DIR +Indicates that the object to be removed is a directory. + +If the current backend context is not in exclusive mode +(@pxref{Exclusive Mode}), this information may be incorrect. +If so, the function should fail with @code{ENOTDIR} or @code{EISDIR}. + @item BOOKMARKFS_BOOKMARK_TYPE_MASK The subsystem to operate on. diff --git a/src/backend_chromium.c b/src/backend_chromium.c index 89f48b3..f19ed1e 100644 --- a/src/backend_chromium.c +++ b/src/backend_chromium.c @@ -2194,7 +2194,7 @@ bookmark_delete ( void *backend_ctx, uint64_t parent_id, char const *name, - uint32_t UNUSED_VAR(flags) + uint32_t flags ) { struct backend_ctx *ctx = backend_ctx; debug_assert(!(ctx->flags & BOOKMARKFS_BACKEND_READONLY)); @@ -2223,15 +2223,19 @@ bookmark_delete ( if (0 != lookup_name(ctx, parent_id, name, strlen(name), &lctx, &entry)) { return -ENOENT; }; - json_t *siblings = parent_entry->children; - json_t const *node = entry->node; // Check if entry can be deleted - json_t const *children = json_object_sget(node, "children"); - if (children != NULL) { - if (0 != json_array_size(children)) { + if (flags & BOOKMARK_FLAG(DELETE_DIR)) { + if (unlikely(entry->children == NULL)) { + return -ENOTDIR; + } + if (0 != json_array_size(entry->children)) { return -ENOTEMPTY; } + } else { + if (unlikely(entry->children != NULL)) { + return -EISDIR; + } } // Remove from maps @@ -2245,7 +2249,8 @@ bookmark_delete ( free(entry); // Remove from store - json_array_remove(siblings, json_array_search(siblings, node)); + json_t *siblings = parent_entry->children; + json_array_remove(siblings, json_array_search(siblings, entry->node)); ctx->dirty = DIRTY_LEVEL_DATA; diff --git a/src/backend_firefox.c b/src/backend_firefox.c index 4b735c3..5ac523c 100644 --- a/src/backend_firefox.c +++ b/src/backend_firefox.c @@ -454,7 +454,7 @@ bookmark_do_delete ( return status; } if (unlikely((cols.place_id == 0) != is_dir)) { - return is_dir ? -ENOTDIR : -EPERM; + return is_dir ? -ENOTDIR : -EISDIR; } status = mozbm_delete(ctx, cols.id, is_dir);