[PATCH] run.in: Detect binaries in builddir properly

Michal Privoznik posted 1 patch 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20970257bf6feaa25bd3e74411ab3fe69a4aaa9d.1716391914.git.mprivozn@redhat.com
There is a newer version of this series
run.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] run.in: Detect binaries in builddir properly
Posted by Michal Privoznik 6 months ago
When attempting to run:

  libvirt.git/_build # ./run --selinux ./src/libvirtd

the following error is thrown:

  Refusing to change selinux context of file './src/libvirtd' outside build directory

which is obviously wrong. The problem is 'being inside of build
directory' is detected by simple progpath.startswith(builddir).
While builddir is an absolute path, progpath isn't necessarily.

And while looking into the code, I've noticed chcon() function
accessing variable outside its scope when printing out the path
it's working on.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 run.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/run.in b/run.in
index 5b89b3dcd5..87cf39a920 100644
--- a/run.in
+++ b/run.in
@@ -138,7 +138,7 @@ def change_unit(name, action):
 
 
 def chcon(path, user, role, type):
-    print("Setting file context of {} to u={}, r={}, t={}...".format(progpath,
+    print("Setting file context of {} to u={}, r={}, t={}...".format(path,
                                                                      user,
                                                                      role,
                                                                      type))
@@ -186,7 +186,7 @@ else:
 
     try:
         dorestorecon = False
-        progpath = shutil.which(prog)
+        progpath = os.path.abspath(shutil.which(prog) or prog)
         if len(try_stop_units):
             print("Temporarily stopping systemd units...")
 
-- 
2.44.1
Re: [PATCH] run.in: Detect binaries in builddir properly
Posted by Jiri Denemark 6 months ago
On Wed, May 22, 2024 at 17:31:54 +0200, Michal Privoznik wrote:
> When attempting to run:
> 
>   libvirt.git/_build # ./run --selinux ./src/libvirtd
> 
> the following error is thrown:
> 
>   Refusing to change selinux context of file './src/libvirtd' outside build directory
> 
> which is obviously wrong. The problem is 'being inside of build
> directory' is detected by simple progpath.startswith(builddir).
> While builddir is an absolute path, progpath isn't necessarily.
> 
> And while looking into the code, I've noticed chcon() function
> accessing variable outside its scope when printing out the path
> it's working on.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  run.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/run.in b/run.in
> index 5b89b3dcd5..87cf39a920 100644
> --- a/run.in
> +++ b/run.in
> @@ -138,7 +138,7 @@ def change_unit(name, action):
>  
>  
>  def chcon(path, user, role, type):
> -    print("Setting file context of {} to u={}, r={}, t={}...".format(progpath,
> +    print("Setting file context of {} to u={}, r={}, t={}...".format(path,
>                                                                       user,
>                                                                       role,
>                                                                       type))
> @@ -186,7 +186,7 @@ else:
>  
>      try:
>          dorestorecon = False
> -        progpath = shutil.which(prog)
> +        progpath = os.path.abspath(shutil.which(prog) or prog)
>          if len(try_stop_units):
>              print("Temporarily stopping systemd units...")

If shutil.which(prog) returns None, progpath should be set to None
rather than an absolute path for prog. Because if prog does not exist
abspath would still construct a path for it. And later on we would fail
to report and error about nonexistent binary.

Jirka