We're seeing failures where a file is renamed under pseudo but an
access appears to be made to the old filename before the OP_RENAME has
hit the database but after the real_rename has applied in the kernel.
This is effectively the MAY_UNLINK problem for the original filename. There
were protections for the newpath but not the oldpath.
To try and avoid these aborts(), mark the original path as MAY_UNLINK however
we need to be careful not to trigger the DID_UNLINK but instead update
the deleting entry in the database as the rename completes. To do this,
we no clear the deleting flag during the database rename operation in SQL.
Also, we have to stop removing the by_ino matches where by_ino.deleting is
set, else we may lose the renamed file's attributes that way.
Signed-off-by: Richard Purdie <richard.pur...@linuxfoundation.org>
---
ports/unix/guts/rename.c | 35 ++++++++++++++++++++++-------------
ports/unix/guts/renameat.c | 34 +++++++++++++++++++++-------------
pseudo.c | 11 +++++------
pseudo_db.c | 4 ++--
4 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/ports/unix/guts/rename.c b/ports/unix/guts/rename.c
index 7e4a499..80bbf41 100644
--- a/ports/unix/guts/rename.c
+++ b/ports/unix/guts/rename.c
@@ -13,7 +13,8 @@
int oldrc, newrc;
int save_errno;
int old_db_entry = 0;
- int may_unlinked = 0;
+ int may_unlink_new = 0;
+ int may_unlink_old = 0;
pseudo_debug(PDBGF_OP, "rename: %s->%s\n",
oldpath ? oldpath : "<nil>",
@@ -43,28 +44,36 @@
/* as with unlink, we have to mark that the file may get deleted */
msg = pseudo_client_op(OP_MAY_UNLINK, 0, -1, -1, newpath, newrc ? NULL
: &newbuf);
if (msg && msg->result == RESULT_SUCCEED)
- may_unlinked = 1;
+ may_unlink_new = 1;
+ /* oldpath is also likely to disappear. Something could call stat()
after
+ real_rename so we need to mark as MAY_UNLINK too */
+ msg = pseudo_client_op(OP_MAY_UNLINK, 0, -1, -1, oldpath, oldrc ? NULL
: &oldbuf);
+ if (msg && msg->result == RESULT_SUCCEED)
+ may_unlink_old = 1;
+
msg = pseudo_client_op(OP_STAT, 0, -1, -1, oldpath, oldrc ? NULL :
&oldbuf);
if (msg && msg->result == RESULT_SUCCEED)
old_db_entry = 1;
rc = real_rename(oldpath, newpath);
save_errno = errno;
- if (may_unlinked) {
- if (rc == -1) {
- /* since we failed, that wasn't really unlinked -- put
- * it back.
- */
- pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, -1, newpath,
&newbuf);
- } else {
- /* confirm that the file was removed */
- pseudo_client_op(OP_DID_UNLINK, 0, -1, -1, newpath,
&newbuf);
- }
- }
+
if (rc == -1) {
+ /* since we failed, that wasn't really unlinked -- put
+ * it back.
+ */
+ if (may_unlink_new)
+ pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, -1, newpath,
&newbuf);
+ if (may_unlink_old)
+ pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, -1, oldpath,
&oldbuf);
/* and we're done. */
errno = save_errno;
return rc;
}
+
+ /* confirm that the file was removed */
+ if (may_unlink_new)
+ pseudo_client_op(OP_DID_UNLINK, 0, -1, -1, newpath, &newbuf);
+ /* OP_DID_UNLINK for oldpath is handled by the server */
save_errno = errno;
/* rename(3) is not mv(1). rename(file, dir) fails; you must provide
diff --git a/ports/unix/guts/renameat.c b/ports/unix/guts/renameat.c
index 8064818..5ac63f9 100644
--- a/ports/unix/guts/renameat.c
+++ b/ports/unix/guts/renameat.c
@@ -13,7 +13,8 @@
int oldrc, newrc;
int save_errno;
int old_db_entry = 0;
- int may_unlinked = 0;
+ int may_unlink_new = 0;
+ int may_unlink_old = 0;
pseudo_debug(PDBGF_FILE, "renameat: %d,%s->%d,%s\n",
olddirfd, oldpath ? oldpath : "<nil>",
@@ -55,29 +56,36 @@
/* as with unlink, we have to mark that the file may get deleted */
msg = pseudo_client_op(OP_MAY_UNLINK, 0, -1, newdirfd, newpath, newrc ?
NULL : &newbuf);
if (msg && msg->result == RESULT_SUCCEED)
- may_unlinked = 1;
+ may_unlink_new = 1;
+ /* oldpath is also likely to disappear. Something could call stat()
after
+ real_rename so we need to mark as MAY_UNLINK too */
+ msg = pseudo_client_op(OP_MAY_UNLINK, 0, -1, olddirfd, oldpath, oldrc ?
NULL : &oldbuf);
+ if (msg && msg->result == RESULT_SUCCEED)
+ may_unlink_old = 1;
+
msg = pseudo_client_op(OP_STAT, 0, -1, olddirfd, oldpath, oldrc ? NULL
: &oldbuf);
if (msg && msg->result == RESULT_SUCCEED)
old_db_entry = 1;
rc = real_renameat(olddirfd, oldpath, newdirfd, newpath);
save_errno = errno;
- if (may_unlinked) {
- if (rc == -1) {
- /* since we failed, that wasn't really unlinked -- put
- * it back.
- */
- pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, newdirfd,
newpath, &newbuf);
- } else {
- /* confirm that the file was removed */
- pseudo_client_op(OP_DID_UNLINK, 0, -1, newdirfd,
newpath, &newbuf);
- }
- }
if (rc == -1) {
+ /* since we failed, that wasn't really unlinked -- put
+ * it back.
+ */
+ if (may_unlink_new)
+ pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, newdirfd,
newpath, &newbuf);
+ if (may_unlink_old)
+ pseudo_client_op(OP_CANCEL_UNLINK, 0, -1, olddirfd,
oldpath, &oldbuf);
/* and we're done. */
errno = save_errno;
return rc;
}
+
+ /* confirm that the file was removed */
+ if (may_unlink_new)
+ pseudo_client_op(OP_DID_UNLINK, 0, -1, newdirfd, newpath,
&newbuf);
+ /* OP_DID_UNLINK for oldpath is handled by the server */
save_errno = errno;
/* rename(3) is not mv(1). rename(file, dir) fails; you must provide
diff --git a/pseudo.c b/pseudo.c
index f2e2f87..528fe1b 100644
--- a/pseudo.c
+++ b/pseudo.c
@@ -688,14 +688,13 @@ pseudo_op(pseudo_msg_t *msg, const char *program, const
char *tag, char **respon
break;
}
if (mismatch) {
- /* a mismatch, but we were planning to delete
- * the file, so it must have gotten deleted
- * already.
- */
if (by_ino.deleting != 0) {
- pseudo_debug(PDBGF_FILE, "inode
mismatch for '%s' -- old one was marked for deletion, deleting.\n",
+ /* a mismatch, likely a race but we
were planning to
+ * delete the file, or are in the
middle of a rename
+ * so continue, need the old entry for
a rename.
+ */
+ pseudo_debug(PDBGF_FILE, "inode
mismatch for '%s' -- old one was marked for deletion.\n",
msg->path);
- pdb_did_unlink_file(path_by_ino,
&by_ino, by_ino.deleting);
} else {
pseudo_diag("path mismatch [%d link%s]:
ino %llu db '%s' req '%s'.\n",
msg->nlink,
diff --git a/pseudo_db.c b/pseudo_db.c
index ebf6e08..6c48cc8 100644
--- a/pseudo_db.c
+++ b/pseudo_db.c
@@ -2020,8 +2020,8 @@ int
pdb_rename_file(const char *oldpath, pseudo_msg_t *msg) {
static sqlite3_stmt *update_exact, *update_sub;
int rc;
- char *sql_update_exact = "UPDATE files SET path = ? WHERE path = ?;";
- char *sql_update_sub = "UPDATE files SET path = replace(path, ?, ?) "
+ char *sql_update_exact = "UPDATE files SET path = ?, deleting = 0 WHERE
path = ?;";
+ char *sql_update_sub = "UPDATE files SET path = replace(path, ?, ?),
deleting = 0 "
"WHERE (path > (? || '/') AND path < (? ||
'0'));";
if (!file_db && get_dbs()) {
--
2.27.0
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#147972):
https://lists.openembedded.org/g/openembedded-core/message/147972
Mute This Topic: https://lists.openembedded.org/mt/80563056/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
