a5adb69dac
This is an automated DistroBaker update from upstream sources. If you do not know what this is about or would like to opt out, contact the OSCI team. Source: https://src.fedoraproject.org/rpms/autofs.git#25aaf0b69441b4e7370a195cbf1c7988d0abef3d
252 lines
7.5 KiB
Diff
252 lines
7.5 KiB
Diff
autofs-5.1.7 - fix inconsistent locking in parse_mount()
|
|
|
|
From: Ian Kent <raven@themaw.net>
|
|
|
|
Some map entry cache locking inconsistencies have crept in.
|
|
|
|
In parse_mount() of the sun format parser the cache read lock is too
|
|
heavily used and has too broad a scope. This has lead to some operations
|
|
that should hold the write lock being called with only the read lock.
|
|
|
|
Signed-off-by: Ian Kent <raven@themaw.net>
|
|
---
|
|
CHANGELOG | 1 +
|
|
lib/mounts.c | 9 ++++++++-
|
|
modules/parse_sun.c | 53 ++++++++++++++++++++++++++++++++-------------------
|
|
3 files changed, 42 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/CHANGELOG b/CHANGELOG
|
|
index c60a9ed3..d25b19c8 100644
|
|
--- a/CHANGELOG
|
|
+++ b/CHANGELOG
|
|
@@ -18,6 +18,7 @@
|
|
- fix inconsistent locking in umount_subtree_mounts().
|
|
- fix return from umount_subtree_mounts() on offset list delete.
|
|
- pass mapent_cache to update_offset_entry().
|
|
+- fix inconsistent locking in parse_mount().
|
|
|
|
25/01/2021 autofs-5.1.7
|
|
- make bind mounts propagation slave by default.
|
|
diff --git a/lib/mounts.c b/lib/mounts.c
|
|
index 5ebfe5fd..0fcd4087 100644
|
|
--- a/lib/mounts.c
|
|
+++ b/lib/mounts.c
|
|
@@ -2491,6 +2491,12 @@ static int do_mount_autofs_offset(struct autofs_point *ap,
|
|
else {
|
|
debug(ap->logopt, "ignoring \"nohide\" trigger %s",
|
|
oe->key);
|
|
+ /*
|
|
+ * Ok, so we shouldn't modify the mapent but
|
|
+ * mount requests are blocked at a point above
|
|
+ * this and expire only uses the mapent key or
|
|
+ * holds the cache write lock.
|
|
+ */
|
|
free(oe->mapent);
|
|
oe->mapent = NULL;
|
|
}
|
|
@@ -2634,7 +2640,8 @@ static int do_umount_offset(struct autofs_point *ap, struct mapent *oe, const ch
|
|
/*
|
|
* Ok, so we shouldn't modify the mapent but
|
|
* mount requests are blocked at a point above
|
|
- * this and expire only uses the mapent key.
|
|
+ * this and expire only uses the mapent key or
|
|
+ * holds the cache write lock.
|
|
*/
|
|
if (oe->mapent) {
|
|
free(oe->mapent);
|
|
diff --git a/modules/parse_sun.c b/modules/parse_sun.c
|
|
index 95251bee..a6630a76 100644
|
|
--- a/modules/parse_sun.c
|
|
+++ b/modules/parse_sun.c
|
|
@@ -851,10 +851,12 @@ update_offset_entry(struct autofs_point *ap,
|
|
strcpy(m_mapent, loc);
|
|
}
|
|
|
|
+ cache_writelock(mc);
|
|
ret = cache_update_offset(mc, name, m_key, m_mapent, age);
|
|
|
|
if (!cache_set_offset_parent(mc, m_key))
|
|
error(ap->logopt, "failed to set offset parent");
|
|
+ cache_unlock(mc);
|
|
|
|
if (ret == CHE_DUPLICATE) {
|
|
warn(ap->logopt, MODPREFIX
|
|
@@ -1128,14 +1130,22 @@ static void cleanup_multi_triggers(struct autofs_point *ap,
|
|
return;
|
|
}
|
|
|
|
-static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
+static int mount_subtree(struct autofs_point *ap, struct mapent_cache *mc,
|
|
const char *name, char *loc, char *options, void *ctxt)
|
|
{
|
|
+ struct mapent *me;
|
|
struct mapent *ro;
|
|
char *mm_root, *mm_base, *mm_key;
|
|
unsigned int mm_root_len;
|
|
int start, ret = 0, rv;
|
|
|
|
+ cache_readlock(mc);
|
|
+ me = cache_lookup_distinct(mc, name);
|
|
+ if (!me) {
|
|
+ cache_unlock(mc);
|
|
+ return 0;
|
|
+ }
|
|
+
|
|
rv = 0;
|
|
|
|
mm_key = me->multi->key;
|
|
@@ -1180,9 +1190,12 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
rv = parse_mapent(ro->mapent,
|
|
options, &myoptions, &ro_loc, ap->logopt);
|
|
if (!rv) {
|
|
+ cache_unlock(mc);
|
|
warn(ap->logopt,
|
|
MODPREFIX "failed to parse root offset");
|
|
- cache_delete_offset_list(me->mc, name);
|
|
+ cache_writelock(mc);
|
|
+ cache_delete_offset_list(mc, name);
|
|
+ cache_unlock(mc);
|
|
return 1;
|
|
}
|
|
ro_len = 0;
|
|
@@ -1199,9 +1212,10 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
if ((ro && rv == 0) || rv <= 0) {
|
|
ret = mount_multi_triggers(ap, me, mm_root, start, mm_base);
|
|
if (ret == -1) {
|
|
+ cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
|
|
+ cache_unlock(mc);
|
|
error(ap->logopt, MODPREFIX
|
|
"failed to mount offset triggers");
|
|
- cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
|
|
return 1;
|
|
}
|
|
}
|
|
@@ -1217,9 +1231,10 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
if (rv == 0) {
|
|
ret = mount_multi_triggers(ap, me->multi, name, start, mm_base);
|
|
if (ret == -1) {
|
|
+ cleanup_multi_triggers(ap, me, name, start, mm_base);
|
|
+ cache_unlock(mc);
|
|
error(ap->logopt, MODPREFIX
|
|
"failed to mount offset triggers");
|
|
- cleanup_multi_triggers(ap, me, name, start, mm_base);
|
|
return 1;
|
|
}
|
|
} else if (rv < 0) {
|
|
@@ -1227,8 +1242,11 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
unsigned int mm_root_base_len = mm_root_len + strlen(mm_base) + 1;
|
|
|
|
if (mm_root_base_len > PATH_MAX) {
|
|
+ cache_unlock(mc);
|
|
warn(ap->logopt, MODPREFIX "path too long");
|
|
- cache_delete_offset_list(me->mc, name);
|
|
+ cache_writelock(mc);
|
|
+ cache_delete_offset_list(mc, name);
|
|
+ cache_unlock(mc);
|
|
return 1;
|
|
}
|
|
|
|
@@ -1237,13 +1255,15 @@ static int mount_subtree(struct autofs_point *ap, struct mapent *me,
|
|
|
|
ret = mount_multi_triggers(ap, me->multi, mm_root_base, start, mm_base);
|
|
if (ret == -1) {
|
|
+ cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
|
|
+ cache_unlock(mc);
|
|
error(ap->logopt, MODPREFIX
|
|
"failed to mount offset triggers");
|
|
- cleanup_multi_triggers(ap, me, mm_root, start, mm_base);
|
|
return 1;
|
|
}
|
|
}
|
|
}
|
|
+ cache_unlock(mc);
|
|
|
|
/* Mount for base of tree failed */
|
|
if (rv > 0)
|
|
@@ -1484,7 +1504,6 @@ dont_expand:
|
|
return 1;
|
|
}
|
|
|
|
- cache_multi_writelock(me);
|
|
/* So we know we're the multi-mount root */
|
|
if (!me->multi)
|
|
me->multi = me;
|
|
@@ -1509,14 +1528,13 @@ dont_expand:
|
|
if (source->flags & MAP_FLAG_FORMAT_AMD) {
|
|
free(options);
|
|
free(pmapent);
|
|
- cache_multi_unlock(me);
|
|
cache_unlock(mc);
|
|
pthread_setcancelstate(cur_state, NULL);
|
|
return 0;
|
|
}
|
|
}
|
|
-
|
|
age = me->age;
|
|
+ cache_unlock(mc);
|
|
|
|
/* It's a multi-mount; deal with it */
|
|
do {
|
|
@@ -1537,8 +1555,8 @@ dont_expand:
|
|
|
|
if (!path) {
|
|
warn(ap->logopt, MODPREFIX "null path or out of memory");
|
|
+ cache_writelock(mc);
|
|
cache_delete_offset_list(mc, name);
|
|
- cache_multi_unlock(me);
|
|
cache_unlock(mc);
|
|
free(options);
|
|
free(pmapent);
|
|
@@ -1554,8 +1572,8 @@ dont_expand:
|
|
|
|
l = parse_mapent(p, options, &myoptions, &loc, ap->logopt);
|
|
if (!l) {
|
|
+ cache_writelock(mc);
|
|
cache_delete_offset_list(mc, name);
|
|
- cache_multi_unlock(me);
|
|
cache_unlock(mc);
|
|
free(path);
|
|
free(options);
|
|
@@ -1573,8 +1591,8 @@ dont_expand:
|
|
|
|
if (status != CHE_OK) {
|
|
warn(ap->logopt, MODPREFIX "error adding multi-mount");
|
|
+ cache_writelock(mc);
|
|
cache_delete_offset_list(mc, name);
|
|
- cache_multi_unlock(me);
|
|
cache_unlock(mc);
|
|
free(path);
|
|
free(options);
|
|
@@ -1592,10 +1610,7 @@ dont_expand:
|
|
free(myoptions);
|
|
} while (*p == '/' || (*p == '"' && *(p + 1) == '/'));
|
|
|
|
- rv = mount_subtree(ap, me, name, NULL, options, ctxt);
|
|
-
|
|
- cache_multi_unlock(me);
|
|
- cache_unlock(mc);
|
|
+ rv = mount_subtree(ap, mc, name, NULL, options, ctxt);
|
|
|
|
free(options);
|
|
free(pmapent);
|
|
@@ -1616,6 +1631,7 @@ dont_expand:
|
|
cache_readlock(mc);
|
|
if (*name == '/' &&
|
|
(me = cache_lookup_distinct(mc, name)) && me->multi) {
|
|
+ cache_unlock(mc);
|
|
loc = strdup(p);
|
|
if (!loc) {
|
|
free(options);
|
|
@@ -1624,10 +1640,7 @@ dont_expand:
|
|
warn(ap->logopt, MODPREFIX "out of memory");
|
|
return 1;
|
|
}
|
|
- cache_multi_writelock(me);
|
|
- rv = mount_subtree(ap, me, name, loc, options, ctxt);
|
|
- cache_multi_unlock(me);
|
|
- cache_unlock(mc);
|
|
+ rv = mount_subtree(ap, mc, name, loc, options, ctxt);
|
|
free(loc);
|
|
free(options);
|
|
free(pmapent);
|