Libguestfs - Mar 2016 - [PATCH] inspect: improve UsrMove detection (RHBZ#1186935)

In case /usr is a symlink to /usr/bin, then we cannot rely on /usr/bin
to exist, since /usr might be in a different partition.  Thus, in case
/bin is a symlink, check it points to "usr/bin".
---
 src/inspect-fs.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 02fdb2a..0714ae1 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -43,6 +43,7 @@ static int check_filesystem (guestfs_h *g, const char
*mountable,
                              int whole_device);
 static void extend_fses (guestfs_h *g);
 static int get_partition_context (guestfs_h *g, const char *partition, int
*partnum_ret, int *nr_partitions_ret);
+static int is_symlink_to (guestfs_h *g, const char *file, const char
*wanted_target);
 
 /* Find out if 'device' contains a filesystem.  If it does, add
  * another entry in g->fses.
@@ -215,8 +216,7 @@ check_filesystem (guestfs_h *g, const char *mountable,
   /* Linux root? */
   else if (is_dir_etc &&
            (is_dir_bin ||
-            (guestfs_is_symlink (g, "/bin") > 0 &&
-             guestfs_is_dir (g, "/usr/bin") > 0)) &&
+            is_symlink_to (g, "/bin", "usr/bin") > 0)
&&
            guestfs_is_file (g, "/etc/fstab") > 0) {
     fs->is_root = 1;
     fs->format = OS_FORMAT_INSTALLED;
@@ -366,6 +366,22 @@ get_partition_context (guestfs_h *g, const char *partition,
   return 0;
 }
 
+static int
+is_symlink_to (guestfs_h *g, const char *file, const char *wanted_target)
+{
+  CLEANUP_FREE char *target = NULL;
+
+  if (guestfs_is_symlink (g, file) == 0)
+    return 0;
+
+  target = guestfs_readlink (g, file);
+  /* This should not fail, but play safe. */
+  if (target == NULL)
+    return 0;
+
+  return STREQ (target, wanted_target);
+}
+
 int
 guestfs_int_is_file_nocase (guestfs_h *g, const char *path)
 {
-- 
2.5.0

On Thu, Mar 17, 2016 at 12:25:18PM +0100, Pino Toscano
wrote:
> In case /usr is a symlink to /usr/bin, then we cannot rely on /usr/bin
> to exist, since /usr might be in a different partition.  Thus, in case
> /bin is a symlink, check it points to "usr/bin".
> ---
>  src/inspect-fs.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/inspect-fs.c b/src/inspect-fs.c
> index 02fdb2a..0714ae1 100644
> --- a/src/inspect-fs.c
> +++ b/src/inspect-fs.c
> @@ -43,6 +43,7 @@ static int check_filesystem (guestfs_h *g, const char
*mountable,
>                               int whole_device);
>  static void extend_fses (guestfs_h *g);
>  static int get_partition_context (guestfs_h *g, const char *partition, int
*partnum_ret, int *nr_partitions_ret);
> +static int is_symlink_to (guestfs_h *g, const char *file, const char
*wanted_target);
>  
>  /* Find out if 'device' contains a filesystem.  If it does, add
>   * another entry in g->fses.
> @@ -215,8 +216,7 @@ check_filesystem (guestfs_h *g, const char *mountable,
>    /* Linux root? */
>    else if (is_dir_etc &&
>             (is_dir_bin ||
> -            (guestfs_is_symlink (g, "/bin") > 0 &&
> -             guestfs_is_dir (g, "/usr/bin") > 0)) &&
> +            is_symlink_to (g, "/bin", "usr/bin") >
0) &&
>             guestfs_is_file (g, "/etc/fstab") > 0) {
>      fs->is_root = 1;
>      fs->format = OS_FORMAT_INSTALLED;
> @@ -366,6 +366,22 @@ get_partition_context (guestfs_h *g, const char
*partition,
>    return 0;
>  }
>  
> +static int
> +is_symlink_to (guestfs_h *g, const char *file, const char *wanted_target)
> +{
> +  CLEANUP_FREE char *target = NULL;
> +
> +  if (guestfs_is_symlink (g, file) == 0)
> +    return 0;
> +
> +  target = guestfs_readlink (g, file);
> +  /* This should not fail, but play safe. */
> +  if (target == NULL)
> +    return 0;
> +
> +  return STREQ (target, wanted_target);
> +}
> +
>  int
>  guestfs_int_is_file_nocase (guestfs_h *g, const char *path)
>  {
Yup, pretty good solution.  ACK.

However, worth checking what Debian is doing now, since they
implemented UsrMove madness just recently.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Thursday 17 March 2016 16:32:58 Richard W.M. Jones
wrote:
> On Thu, Mar 17, 2016 at 12:25:18PM +0100, Pino Toscano wrote:
> > In case /usr is a symlink to /usr/bin, then we cannot rely on /usr/bin
> > to exist, since /usr might be in a different partition.  Thus, in case
> > /bin is a symlink, check it points to "usr/bin".
> > ---
> >  src/inspect-fs.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/inspect-fs.c b/src/inspect-fs.c
> > index 02fdb2a..0714ae1 100644
> > --- a/src/inspect-fs.c
> > +++ b/src/inspect-fs.c
> > @@ -43,6 +43,7 @@ static int check_filesystem (guestfs_h *g, const
char *mountable,
> >                               int whole_device);
> >  static void extend_fses (guestfs_h *g);
> >  static int get_partition_context (guestfs_h *g, const char
*partition, int *partnum_ret, int *nr_partitions_ret);
> > +static int is_symlink_to (guestfs_h *g, const char *file, const char
*wanted_target);
> >  
> >  /* Find out if 'device' contains a filesystem.  If it does,
add
> >   * another entry in g->fses.
> > @@ -215,8 +216,7 @@ check_filesystem (guestfs_h *g, const char
*mountable,
> >    /* Linux root? */
> >    else if (is_dir_etc &&
> >             (is_dir_bin ||
> > -            (guestfs_is_symlink (g, "/bin") > 0
&&
> > -             guestfs_is_dir (g, "/usr/bin") > 0))
&&
> > +            is_symlink_to (g, "/bin", "usr/bin")
> 0) &&
> >             guestfs_is_file (g, "/etc/fstab") > 0) {
> >      fs->is_root = 1;
> >      fs->format = OS_FORMAT_INSTALLED;
> > @@ -366,6 +366,22 @@ get_partition_context (guestfs_h *g, const char
*partition,
> >    return 0;
> >  }
> >  
> > +static int
> > +is_symlink_to (guestfs_h *g, const char *file, const char
*wanted_target)
> > +{
> > +  CLEANUP_FREE char *target = NULL;
> > +
> > +  if (guestfs_is_symlink (g, file) == 0)
> > +    return 0;
> > +
> > +  target = guestfs_readlink (g, file);
> > +  /* This should not fail, but play safe. */
> > +  if (target == NULL)
> > +    return 0;
> > +
> > +  return STREQ (target, wanted_target);
> > +}
> > +
> >  int
> >  guestfs_int_is_file_nocase (guestfs_h *g, const char *path)
> >  {
> 
> Yup, pretty good solution.  ACK.
> 
> However, worth checking what Debian is doing now, since they
> implemented UsrMove madness just recently.
I just tested it on a Debian/unstable VM and installing usrmerge, as
also described in https://wiki.debian.org/UsrMerge - the detection
worked.

Hence pushing -- thanks!

-- 
Pino Toscano