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

Michal Privoznik posted 1 patch 4 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/7245522a0cd115f1154246cdf6b9d7aff373063e.1716886077.git.mprivozn@redhat.com
run.in | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH v2] run.in: Detect binaries in builddir properly
Posted by Michal Privoznik 4 months, 3 weeks 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>
---

v2 of:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UZ6F7MWEJBUMUUBODXGAXQW4NY2UEEVF/

diff to v1:
- error out if binary to run can't be identified (i.e. 'which' returns
  None).

 run.in | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/run.in b/run.in
index 5b89b3dcd5..cada74dfcd 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))
@@ -187,6 +187,10 @@ else:
     try:
         dorestorecon = False
         progpath = shutil.which(prog)
+        if not progpath:
+            raise Exception("Can't find executable {}"
+                            .format(prog))
+        progpath = os.path.abspath(progpath)
         if len(try_stop_units):
             print("Temporarily stopping systemd units...")
 
-- 
2.44.1
Re: [PATCH v2] run.in: Detect binaries in builddir properly
Posted by Jiri Denemark 4 months, 3 weeks ago
On Tue, May 28, 2024 at 10:49:34 +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>
> ---
> 
> v2 of:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UZ6F7MWEJBUMUUBODXGAXQW4NY2UEEVF/
> 
> diff to v1:
> - error out if binary to run can't be identified (i.e. 'which' returns
>   None).
> 
>  run.in | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/run.in b/run.in
> index 5b89b3dcd5..cada74dfcd 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))
> @@ -187,6 +187,10 @@ else:
>      try:
>          dorestorecon = False
>          progpath = shutil.which(prog)
> +        if not progpath:
> +            raise Exception("Can't find executable {}"
> +                            .format(prog))
> +        progpath = os.path.abspath(progpath)
>          if len(try_stop_units):
>              print("Temporarily stopping systemd units...")

You can drop the second (now unreachable) check a few lines later:

    if not progpath:
        raise Exception("Can't find executable {} for selinux labeling"
                        .format(prog))


Reviewed-by: Jiri Denemark <jdenemar@redhat.com>