libmount: mnt_resolve_target: tiptoe around active mount points

Current code in mnt_fs_match_target() and mnt_table_find_target()
already does not canonicalize active mount points (when read from
mountinfo), because they are already canonicalized by the kernel.
Calling realpath(fs->target) on a mount point can hang -- e.g. if the
NFS server is unreachable.

This patch optionally extends this strategy to the general case, that is
when @fs does not directly come from the kernel through mountinfo (for
instance, it may have been parsed from /etc/fstab).

Given @mtab parsed from mountinfo, and if mnt_cache_set_targets(cache,
mtab) is used, then mnt_fs_match_target() and mnt_table_find_target()
check whether @fs->target is a known mount point in the cached
mountinfo, before attempting to canonicalize @fs->target, no matter
where @fs itself comes from. If found in the cached mountinfo,
@fs->target is not canonicalized.

[kzak@redhat.com: - don't allocate libmnt_iter,
                  - add docs for mnt_cache_set_targets(),
                  - fallback to mnt_resolve_path() if no cache->mtab specified,
                  - use streq_except_trailing_slash() to compare paths]

Signed-off-by: Eric Rannaud <e@nanocritical.com>
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/cache.c b/libmount/src/cache.c
index 2fe41b3a4e87219baea005f9a63cedacbd58f2d6..05b078b4451a5ce7417b0d260ab59088bccc8bd3 100644 (file)
--- a/libmount/src/cache.c
+++ b/libmount/src/cache.c
@@ -27,6 +27,7 @@
 #include "canonicalize.h"
 #include "mountP.h"
 #include "loopdev.h"
+#include "strutils.h"
 
 /*
  * Canonicalized (resolved) paths & tags cache
@@ -60,6 +61,8 @@ struct libmnt_cache {
         *    better to reuse the blkid_cache.
         */
        blkid_cache             bc;
+
+       struct libmnt_table     *mtab;
 };
 
 /**
@@ -131,11 +134,37 @@ void mnt_unref_cache(struct libmnt_cache *cache)
        if (cache) {
                cache->refcount--;
                /*DBG(CACHE, ul_debugobj(cache, "unref=%d", cache->refcount));*/
-               if (cache->refcount <= 0)
+               if (cache->refcount <= 0) {
+                       mnt_unref_table(cache->mtab);
+
                        mnt_free_cache(cache);
+               }
        }
 }
 
+/**
+ * mnt_cache_set_targets:
+ * @cache: cache pointer
+ * @mtab: table with already canonicalized mountpoints
+ *
+ * Add to @cache reference to @mtab. This allows to avoid unnecessary paths
+ * canonicalization in mnt_resolve_target().
+ *
+ * Returns: negative number in case of error, or 0 o success.
+ */
+int mnt_cache_set_targets(struct libmnt_cache *cache,
+                               struct libmnt_table *mtab)
+{
+       assert(cache);
+       if (!cache)
+               return -EINVAL;
+
+       mnt_ref_table(mtab);
+       mnt_unref_table(cache->mtab);
+       cache->mtab = mtab;
+       return 0;
+}
+
 
 /* note that the @key could be the same pointer as @value */
 static int cache_add_entry(struct libmnt_cache *cache, char *key,
@@ -223,7 +252,7 @@ static const char *cache_find_path(struct libmnt_cache *cache, const char *path)
                struct mnt_cache_entry *e = &cache->ents[i];
                if (!(e->flag & MNT_CACHE_ISPATH))
                        continue;
-               if (strcmp(path, e->key) == 0)
+               if (streq_except_trailing_slash(path, e->key))
                        return e->value;
        }
        return NULL;
@@ -468,6 +497,35 @@ char *mnt_get_fstype(const char *devname, int *ambi, struct libmnt_cache *cache)
        return type;
 }
 
+static char *canonicalize_path_and_cache(const char *path,
+                                               struct libmnt_cache *cache)
+{
+       char *p = NULL;
+       char *key = NULL;
+       char *value = NULL;
+
+       p = canonicalize_path(path);
+
+       if (p && cache) {
+               value = p;
+               key = strcmp(path, p) == 0 ? value : strdup(path);
+
+               if (!key || !value)
+                       goto error;
+
+               if (cache_add_entry(cache, key, value,
+                               MNT_CACHE_ISPATH))
+                       goto error;
+       }
+
+       return p;
+error:
+       if (value != key)
+               free(value);
+       free(key);
+       return NULL;
+}
+
 /**
  * mnt_resolve_path:
  * @path: "native" path
@@ -483,8 +541,6 @@ char *mnt_get_fstype(const char *devname, int *ambi, struct libmnt_cache *cache)
 char *mnt_resolve_path(const char *path, struct libmnt_cache *cache)
 {
        char *p = NULL;
-       char *key = NULL;
-       char *value = NULL;
 
        /*DBG(CACHE, ul_debugobj(cache, "resolving path %s", path));*/
 
@@ -492,29 +548,67 @@ char *mnt_resolve_path(const char *path, struct libmnt_cache *cache)
                return NULL;
        if (cache)
                p = (char *) cache_find_path(cache, path);
+       if (!p)
+               p = canonicalize_path_and_cache(path, cache);
 
-       if (!p) {
-               p = canonicalize_path(path);
+       return p;
+}
 
-               if (p && cache) {
-                       value = p;
-                       key = strcmp(path, p) == 0 ? value : strdup(path);
+/**
+ * mnt_resolve_target:
+ * @path: "native" path, a potential mount point
+ * @cache: cache for results or NULL.
+ *
+ * Like mnt_resolve_path(), unless @cache is not NULL and
+ * mnt_cache_set_targets(cache, mtab) was called: if @path is found in the
+ * cached @mtab and the matching entry was provided by the kernel, assume that
+ * @path is already canonicalized. By avoiding a call to canonicalize_path() on
+ * known mount points, there is a lower risk of stepping on a stale mount
+ * point, which can result in an application freeze. This is also faster in
+ * general, as stat(2) on a mount point is slower than on a regular file.
+ *
+ * Returns: absolute path or NULL in case of error. The result has to be
+ * deallocated by free() if @cache is NULL.
+ */
+char *mnt_resolve_target(const char *path, struct libmnt_cache *cache)
+{
+       char *p = NULL;
 
-                       if (!key || !value)
-                               goto error;
+       /*DBG(CACHE, ul_debugobj(cache, "resolving target %s", path));*/
 
-                       if (cache_add_entry(cache, key, value,
-                                                       MNT_CACHE_ISPATH))
-                               goto error;
+       if (!cache || !cache->mtab)
+               return mnt_resolve_path(path, cache);
+
+       p = (char *) cache_find_path(cache, path);
+       if (p)
+               return p;
+       else {
+               struct libmnt_iter itr;
+               struct libmnt_fs *fs = NULL;
+
+               mnt_reset_iter(&itr, MNT_ITER_BACKWARD);
+               while (mnt_table_next_fs(cache->mtab, &itr, &fs) == 0) {
+
+                       if (!mnt_fs_is_kernel(fs)
+                            || mnt_fs_is_swaparea(fs)
+                            || !mnt_fs_streq_target(fs, path))
+                               continue;
+
+                       p = strdup(path);
+                       if (!p)
+                               return NULL;    /* ENOMEM */
+
+                       if (cache_add_entry(cache, p, p, MNT_CACHE_ISPATH)) {
+                               free(p);
+                               return NULL;    /* ENOMEM */
+                       }
+                       break;
                }
        }
 
+       if (!p)
+               p = canonicalize_path_and_cache(path, cache);
        return p;
-error:
-       if (value != key)
-               free(value);
-       free(key);
-       return NULL;
 }
 
 /**

diff --git a/libmount/src/fs.c b/libmount/src/fs.c
index 21ef0f7479fd1455d0d8df5c8695359324a6cefa..82541083a79223b3a1711c60b0e1f23a8e5afea9 100644 (file)
--- a/libmount/src/fs.c
+++ b/libmount/src/fs.c
@@ -1413,7 +1413,9 @@ int mnt_fs_append_comment(struct libmnt_fs *fs, const char *comm)
  *     1) compare @target with @fs->target
  *     2) realpath(@target) with @fs->target
  *     3) realpath(@target) with realpath(@fs->target) if @fs is not from
- *        /proc/self/mountinfo.
+ *        /proc/self/mountinfo. However, if mnt_cache_set_targets(cache,
+ *        mtab) was called, and the path @fs->target is found in @mtab,
+ *        this comparison is not performed (see mnt_resolve_target()).
  *
  * The 2nd and 3rd attempts are not performed when @cache is NULL.
  *
@@ -1438,7 +1440,7 @@ int mnt_fs_match_target(struct libmnt_fs *fs, const char *target,
 
                /* 3) - canonicalized and canonicalized */
                if (!rc && cn && !mnt_fs_is_kernel(fs) && !mnt_fs_is_swaparea(fs)) {
-                       char *tcn = mnt_resolve_path(fs->target, cache);
+                       char *tcn = mnt_resolve_target(fs->target, cache);
                        rc = (tcn && strcmp(cn, tcn) == 0);
                }
        }

diff --git a/libmount/src/libmount.h.in b/libmount/src/libmount.h.in
index 08ddd659600d99ddd69985b2e2a5ef5a489a47f8..ac0c790f2abe8266cb3ed23e6fc9b869eca971b9 100644 (file)
--- a/libmount/src/libmount.h.in
+++ b/libmount/src/libmount.h.in
@@ -220,6 +220,8 @@ extern void mnt_free_cache(struct libmnt_cache *cache);
 extern void mnt_ref_cache(struct libmnt_cache *cache);
 extern void mnt_unref_cache(struct libmnt_cache *cache);
 
+extern int mnt_cache_set_targets(struct libmnt_cache *cache,
+                               struct libmnt_table *mtab);
 extern int mnt_cache_read_tags(struct libmnt_cache *cache, const char *devname);
 
 extern int mnt_cache_device_has_tag(struct libmnt_cache *cache,
@@ -235,6 +237,8 @@ extern char *mnt_get_fstype(const char *devname, int *ambi,
                        __ul_attribute__((warn_unused_result));
 extern char *mnt_resolve_path(const char *path, struct libmnt_cache *cache)
                        __ul_attribute__((warn_unused_result));
+extern char *mnt_resolve_target(const char *path, struct libmnt_cache *cache)
+                       __ul_attribute__((warn_unused_result));
 extern char *mnt_resolve_tag(const char *token, const char *value,
                             struct libmnt_cache *cache)
                        __ul_attribute__((warn_unused_result));

diff --git a/libmount/src/libmount.sym b/libmount/src/libmount.sym
index 088db77494343e1e40fa6714020067c57ff7a8d8..d827c310139db284d1c08f7c37de49623350f199 100644 (file)
--- a/libmount/src/libmount.sym
+++ b/libmount/src/libmount.sym
@@ -289,6 +289,8 @@ global:
 } MOUNT_2.23;
 
 MOUNT_2.25 {
+       mnt_cache_set_targets;
+       mnt_resolve_target;
        mnt_table_uniq_fs;
        mnt_tag_is_valid;
 } MOUNT_2.24;

diff --git a/libmount/src/tab.c b/libmount/src/tab.c
index 77260ab96c35327aa61f172eeb8b4c90358125ef..fd6a7d8e2451253104d9ad805c5db035751ac058 100644 (file)
--- a/libmount/src/tab.c
+++ b/libmount/src/tab.c
@@ -880,8 +880,10 @@ struct libmnt_fs *mnt_table_find_mountpoint(struct libmnt_table *tb,
  *
  * Try to lookup an entry in the given tab, three iterations are possible, the first
  * with @path, the second with realpath(@path) and the third with realpath(@path)
- * against realpath(fs->target). The 2nd and 3rd iterations are not performed
- * when the @tb cache is not set (see mnt_table_set_cache()).
+ * against realpath(fs->target). The 2nd and 3rd iterations are not performed when
+ * the @tb cache is not set (see mnt_table_set_cache()). If
+ * mnt_cache_set_targets(cache, mtab) was called, the 3rd iteration skips any
+ * @fs->target found in @mtab (see mnt_resolve_target()).
  *
  * Returns: a tab entry or NULL.
  */
@@ -933,7 +935,7 @@ struct libmnt_fs *mnt_table_find_target(struct libmnt_table *tb, const char *pat
                    || (*fs->target == '/' && *(fs->target + 1) == '\0'))
                       continue;
 
-               p = mnt_resolve_path(fs->target, tb->cache);
+               p = mnt_resolve_target(fs->target, tb->cache);
                /* both canonicalized, strcmp() is fine here */
                if (p && strcmp(cn, p) == 0)
                        return fs;