[PATCH] utils: Canonicalize paths before comparing them

Andrea Bolognani posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20250217164451.90766-1-abologna@redhat.com
src/util/virfile.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
[PATCH] utils: Canonicalize paths before comparing them
Posted by Andrea Bolognani 10 months ago
Resolves: https://issues.redhat.com/browse/RHEL-79165
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/util/virfile.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6ac0f4efb3..7cab3d0cd6 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3823,10 +3823,13 @@ virFileIsSharedFSOverride(const char *path,
     if (!path || path[0] != '/' || !overrides)
         return false;
 
-    if (g_strv_contains((const char *const *) overrides, path))
-        return true;
+    /* Overrides have been canonicalized ahead of time, so we need to
+     * do the same for the provided path or we'll never be able to
+     * find a match if symlinks are involved */
+    dirpath = virFileCanonicalizePath(path);
 
-    dirpath = g_strdup(path);
+    if (g_strv_contains((const char *const *) overrides, dirpath))
+        return true;
 
     /* Continue until we've scanned the entire path */
     while (p != dirpath) {
-- 
2.48.1
Re: [PATCH] utils: Canonicalize paths before comparing them
Posted by Daniel P. Berrangé 10 months ago
On Mon, Feb 17, 2025 at 05:44:51PM +0100, Andrea Bolognani wrote:
> Resolves: https://issues.redhat.com/browse/RHEL-79165

Please explain the problem in the commit message. Issue tracker links
in general should not be relied upon as explanation, as they tend to
end up going away over time. The issue links are merely convenient
metadata.

> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/util/virfile.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 6ac0f4efb3..7cab3d0cd6 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3823,10 +3823,13 @@ virFileIsSharedFSOverride(const char *path,
>      if (!path || path[0] != '/' || !overrides)
>          return false;
>  
> -    if (g_strv_contains((const char *const *) overrides, path))
> -        return true;
> +    /* Overrides have been canonicalized ahead of time, so we need to
> +     * do the same for the provided path or we'll never be able to
> +     * find a match if symlinks are involved */
> +    dirpath = virFileCanonicalizePath(path);
>  
> -    dirpath = g_strdup(path);
> +    if (g_strv_contains((const char *const *) overrides, dirpath))
> +        return true;
>  
>      /* Continue until we've scanned the entire path */
>      while (p != dirpath) {

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] utils: Canonicalize paths before comparing them
Posted by Andrea Bolognani 10 months ago
On Mon, Feb 17, 2025 at 04:49:17PM +0000, Daniel P. Berrangé wrote:
> On Mon, Feb 17, 2025 at 05:44:51PM +0100, Andrea Bolognani wrote:
> > Resolves: https://issues.redhat.com/browse/RHEL-79165
>
> Please explain the problem in the commit message. Issue tracker links
> in general should not be relied upon as explanation, as they tend to
> end up going away over time. The issue links are merely convenient
> metadata.

You're right, I figured that the code comment I've introduced would
be enough but a slightly more verbose explanation in the commit
message can't possibly hurt. I've added it and pushed. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization