rpms/util-linux
7
0
Fork 0
Code Issues Pull Requests Releases Activity
6f0dbde58a
util-linux/0072-libmount-cleanup-locking-in-table-update-code.patch
Karel Zak 6f0dbde58a RHEL-9.4.0: lscpu, logger, libblkid, libmount-monitor (2.37.4-16) 
Resolves: RHEL-12783 RHEL-14612 RHEL-16048 RHEL-16071 RHEL-21257
2024-02-07 11:52:19 +01:00

353 lines
9.8 KiB
Diff
Raw Blame History

From 317add745924e5a4ad186691eede19d5a7c5eb10 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Fri, 19 Jan 2024 12:58:50 +0100
Subject: libmount: cleanup locking in table update code
* use reference counting for libmnt_lock
* remove lock use from mnt_update_already_done(), it's read-only
operation and utab update is atomic (based on rename(2))
* keep the lock instance mandatory for all low-level update_*
functions. The lock is always initialized, so all the 'if(lc)'
are unnecessary.
Addresses: https://issues.redhat.com/browse/RHEL-14612
Upstream: http://github.com/util-linux/util-linux/commit/b94b5929a63df9786020e0329e94d2f35f3993ae
Signed-off-by: Karel Zak <kzak@redhat.com>
---
libmount/src/context.c | 2 +-
libmount/src/mountP.h | 3 +-
libmount/src/tab_update.c | 118 +++++++++++++++++---------------------
3 files changed, 56 insertions(+), 67 deletions(-)
diff --git a/libmount/src/context.c b/libmount/src/context.c
index ca7ea70d0..06ac50dc2 100644
--- a/libmount/src/context.c
+++ b/libmount/src/context.c
@@ -2290,7 +2290,7 @@ int mnt_context_update_tabs(struct libmnt_context *cxt)
&& mnt_context_get_helper_status(cxt) == 0
&& mnt_context_utab_writable(cxt)) {
- if (mnt_update_already_done(cxt->update, cxt->lock)) {
+ if (mnt_update_already_done(cxt->update)) {
DBG(CXT, ul_debugobj(cxt, "don't update: error evaluate or already updated"));
goto emit;
}
diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h
index 99b913c99..eddc82bde 100644
--- a/libmount/src/mountP.h
+++ b/libmount/src/mountP.h
@@ -477,8 +477,7 @@ extern int mnt_context_deferred_delete_veritydev(struct libmnt_context *cxt);
extern int mnt_update_set_filename(struct libmnt_update *upd,
const char *filename, int userspace_only);
extern int mnt_update_emit_event(struct libmnt_update *upd);
-extern int mnt_update_already_done(struct libmnt_update *upd,
- struct libmnt_lock *lc);
+extern int mnt_update_already_done(struct libmnt_update *upd);
#if __linux__
/* btrfs.c */
diff --git a/libmount/src/tab_update.c b/libmount/src/tab_update.c
index 70013a205..25a734e9d 100644
--- a/libmount/src/tab_update.c
+++ b/libmount/src/tab_update.c
@@ -42,6 +42,7 @@ struct libmnt_update {
missing_options : 1;
struct libmnt_table *mountinfo;
+ struct libmnt_lock *lock;
};
static int set_fs_root(struct libmnt_update *upd, struct libmnt_fs *fs, unsigned long mountflags);
@@ -77,6 +78,7 @@ void mnt_free_update(struct libmnt_update *upd)
DBG(UPDATE, ul_debugobj(upd, "free"));
+ mnt_unref_lock(upd->lock);
mnt_unref_fs(upd->fs);
mnt_unref_table(upd->mountinfo);
free(upd->target);
@@ -691,18 +693,18 @@ static int add_file_entry(struct libmnt_table *tb, struct libmnt_update *upd)
return update_table(upd, tb);
}
-static int update_add_entry(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_add_entry(struct libmnt_update *upd)
{
struct libmnt_table *tb;
int rc = 0;
assert(upd);
assert(upd->fs);
+ assert(upd->lock);
DBG(UPDATE, ul_debugobj(upd, "%s: add entry", upd->filename));
- if (lc)
- rc = mnt_lock_file(lc);
+ rc = mnt_lock_file(upd->lock);
if (rc)
return -MNT_ERR_LOCK;
@@ -710,25 +712,24 @@ static int update_add_entry(struct libmnt_update *upd, struct libmnt_lock *lc)
upd->userspace_only ? MNT_FMT_UTAB : MNT_FMT_MTAB, 1);
if (tb)
rc = add_file_entry(tb, upd);
- if (lc)
- mnt_unlock_file(lc);
+ mnt_unlock_file(upd->lock);
mnt_unref_table(tb);
return rc;
}
-static int update_remove_entry(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_remove_entry(struct libmnt_update *upd)
{
struct libmnt_table *tb;
int rc = 0;
assert(upd);
assert(upd->target);
+ assert(upd->lock);
DBG(UPDATE, ul_debugobj(upd, "%s: remove entry", upd->filename));
- if (lc)
- rc = mnt_lock_file(lc);
+ rc = mnt_lock_file(upd->lock);
if (rc)
return -MNT_ERR_LOCK;
@@ -741,23 +742,22 @@ static int update_remove_entry(struct libmnt_update *upd, struct libmnt_lock *lc
rc = update_table(upd, tb);
}
}
- if (lc)
- mnt_unlock_file(lc);
+ mnt_unlock_file(upd->lock);
mnt_unref_table(tb);
return rc;
}
-static int update_modify_target(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_modify_target(struct libmnt_update *upd)
{
struct libmnt_table *tb = NULL;
int rc = 0;
assert(upd);
+ assert(upd->lock);
DBG(UPDATE, ul_debugobj(upd, "%s: modify target", upd->filename));
- if (lc)
- rc = mnt_lock_file(lc);
+ rc = mnt_lock_file(upd->lock);
if (rc)
return -MNT_ERR_LOCK;
@@ -773,15 +773,13 @@ static int update_modify_target(struct libmnt_update *upd, struct libmnt_lock *l
}
}
- if (lc)
- mnt_unlock_file(lc);
-
+ mnt_unlock_file(upd->lock);
mnt_unref_table(tb);
return rc;
}
/* replaces option in the table entry by new options from @udp */
-static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_modify_options(struct libmnt_update *upd)
{
struct libmnt_table *tb = NULL;
int rc = 0;
@@ -789,13 +787,13 @@ static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *
assert(upd);
assert(upd->fs);
+ assert(upd->lock);
DBG(UPDATE, ul_debugobj(upd, "%s: modify options", upd->filename));
fs = upd->fs;
- if (lc)
- rc = mnt_lock_file(lc);
+ rc = mnt_lock_file(upd->lock);
if (rc)
return -MNT_ERR_LOCK;
@@ -816,15 +814,13 @@ static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *
rc = add_file_entry(tb, upd); /* not found, add new */
}
- if (lc)
- mnt_unlock_file(lc);
-
+ mnt_unlock_file(upd->lock);
mnt_unref_table(tb);
return rc;
}
/* add user options missing in the table entry by new options from @udp */
-static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_add_options(struct libmnt_update *upd)
{
struct libmnt_table *tb = NULL;
int rc = 0;
@@ -832,6 +828,7 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
assert(upd);
assert(upd->fs);
+ assert(upd->lock);
if (!upd->fs->user_optstr)
return 0;
@@ -840,8 +837,7 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
fs = upd->fs;
- if (lc)
- rc = mnt_lock_file(lc);
+ rc = mnt_lock_file(upd->lock);
if (rc)
return -MNT_ERR_LOCK;
@@ -867,14 +863,33 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
rc = add_file_entry(tb, upd); /* not found, add new */
}
- if (lc)
- mnt_unlock_file(lc);
-
+ mnt_unlock_file(upd->lock);
mnt_unref_table(tb);
return rc;
}
+static int update_init_lock(struct libmnt_update *upd, struct libmnt_lock *lc)
+{
+ assert(upd);
+
+ if (lc) {
+ mnt_unref_lock(upd->lock);
+ mnt_ref_lock(lc);
+ upd->lock = lc;
+ } else if (!upd->lock) {
+ upd->lock = mnt_new_lock(upd->filename, 0);
+ if (!upd->lock)
+ return -ENOMEM;
+ mnt_lock_block_signals(upd->lock, TRUE);
+ }
+
+ if (upd->userspace_only)
+ mnt_lock_use_simplelock(upd->lock, TRUE); /* use flock */
+
+ return 0;
+}
+
/**
* mnt_update_table:
* @upd: update
@@ -891,7 +906,6 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
*/
int mnt_update_table(struct libmnt_update *upd, struct libmnt_lock *lc)
{
- struct libmnt_lock *lc0 = lc;
int rc = -EINVAL;
if (!upd || !upd->filename)
@@ -903,37 +917,32 @@ int mnt_update_table(struct libmnt_update *upd, struct libmnt_lock *lc)
if (upd->fs) {
DBG(UPDATE, mnt_fs_print_debug(upd->fs, stderr));
}
- if (!lc) {
- lc = mnt_new_lock(upd->filename, 0);
- if (lc)
- mnt_lock_block_signals(lc, TRUE);
- }
- if (lc && upd->userspace_only)
- mnt_lock_use_simplelock(lc, TRUE); /* use flock */
+
+ rc = update_init_lock(upd, lc);
+ if (rc)
+ goto done;
if (!upd->fs && upd->target)
- rc = update_remove_entry(upd, lc); /* umount */
+ rc = update_remove_entry(upd); /* umount */
else if (upd->mountflags & MS_MOVE)
- rc = update_modify_target(upd, lc); /* move */
+ rc = update_modify_target(upd); /* move */
else if (upd->mountflags & MS_REMOUNT)
- rc = update_modify_options(upd, lc); /* remount */
+ rc = update_modify_options(upd); /* remount */
else if (upd->fs && upd->missing_options)
- rc = update_add_options(upd, lc); /* mount by externel helper */
+ rc = update_add_options(upd); /* mount by externel helper */
else if (upd->fs)
- rc = update_add_entry(upd, lc); /* mount */
+ rc = update_add_entry(upd); /* mount */
upd->ready = 1;
+done:
DBG(UPDATE, ul_debugobj(upd, "%s: update tab: done [rc=%d]",
upd->filename, rc));
- if (lc != lc0)
- mnt_free_lock(lc);
return rc;
}
-int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
+int mnt_update_already_done(struct libmnt_update *upd)
{
struct libmnt_table *tb = NULL;
- struct libmnt_lock *lc0 = lc;
int rc = 0;
if (!upd || !upd->filename || (!upd->fs && !upd->target))
@@ -941,25 +950,8 @@ int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
DBG(UPDATE, ul_debugobj(upd, "%s: checking for previous update", upd->filename));
- if (!lc) {
- lc = mnt_new_lock(upd->filename, 0);
- if (lc)
- mnt_lock_block_signals(lc, TRUE);
- }
- if (lc && upd->userspace_only)
- mnt_lock_use_simplelock(lc, TRUE); /* use flock */
- if (lc) {
- rc = mnt_lock_file(lc);
- if (rc) {
- rc = -MNT_ERR_LOCK;
- goto done;
- }
- }
-
tb = __mnt_new_table_from_file(upd->filename,
upd->userspace_only ? MNT_FMT_UTAB : MNT_FMT_MTAB, 1);
- if (lc)
- mnt_unlock_file(lc);
if (!tb)
goto done;
@@ -996,8 +988,6 @@ int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
mnt_unref_table(tb);
done:
- if (lc && lc != lc0)
- mnt_free_lock(lc);
DBG(UPDATE, ul_debugobj(upd, "%s: previous update check done [rc=%d]",
upd->filename, rc));
return rc;
--
2.43.0
Reference in New Issue View Git Blame Copy Permalink