[Libguestfs] [PATCH] inspection: add support for systemd .mount files
Richard W.M. Jones
rjones at redhat.com
Mon Mar 23 13:23:58 UTC 2015
On Mon, Mar 16, 2015 at 07:09:51PM +0100, Maros Zatko wrote:
> Fixes RHBZ#1113153.
> ---
> src/inspect-fs-unix.c | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 240 insertions(+)
>
> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
> index 2abbf24..6dfc299 100644
> --- a/src/inspect-fs-unix.c
> +++ b/src/inspect-fs-unix.c
> @@ -96,6 +96,9 @@ static char *resolve_fstab_device (guestfs_h *g, const char *spec,
> Hash_table *md_map,
> enum inspect_os_type os_type);
> static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char **configfiles, int (*f) (guestfs_h *, struct inspect_fs *));
> +static int inspect_with_augeas2 (guestfs_h *g, struct inspect_fs *fs, const char **configfiles, int (*f) (guestfs_h *, struct inspect_fs *, const char *));
I think there is a case for splitting out the commit which adds
this new function.
Also, do we need a new function? I don't see why it's needed at all -
just modify the old inspect_with_augeas so it passes the extra
parameter always, and update the call sites [in a separate commit].
> +static int check_systemd_mounts (guestfs_h *g, struct inspect_fs *fs);
> +static int check_systemd_mnt (guestfs_h *g, struct inspect_fs *fs, const char *path);
You don't need to prototype check_systemd_mnt at all - just remove
that line as it's confusing for people reading the code.
> /* Hash structure for uuid->path lookups */
> typedef struct md_uuid {
> @@ -582,6 +585,9 @@ guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs)
> if (check_hostname_unix (g, fs) == -1)
> return -1;
>
> + if (check_systemd_mounts (g, fs) == -1)
> + return -1;
> +
> return 0;
> }
>
> @@ -981,6 +987,180 @@ check_hostname_freebsd (guestfs_h *g, struct inspect_fs *fs)
> }
>
> static int
> +check_systemd_mnt (guestfs_h *g, struct inspect_fs *fs, const char *fname)
> +{
> + CLEANUP_FREE_STRING_LIST char **entries = NULL;
> + char **entry;
> + char augpath[256];
> + CLEANUP_HASH_FREE Hash_table *md_map = NULL;
> + bool is_bsd = (fs->type == OS_TYPE_FREEBSD ||
> + fs->type == OS_TYPE_NETBSD ||
> + fs->type == OS_TYPE_OPENBSD);
> +
> + /* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device
> + * paths in the guestfs appliance */
> + if (map_md_devices (g, &md_map) == -1) {
> + printf ("map_md_dev fail\n");
This printf shouldn't be here.
> + return -1;
> + }
> +
> + char expr[512];
> + snprintf (expr, sizeof expr, "/files%s", fname);
Yikes. Don't use fixed size buffers, especially where (as seems to be
the case here) the path comes from user input. Use:
CLEANUP_FREE char *expr = NULL;
...
expr = safe_asprintf (g, "/files%s", fname);
> + entries = guestfs_aug_match (g, expr);
> + if (entries == NULL) {
> + return 0;
> + }
> +
> + for (entry = entries; *entry != NULL; entry++) {
> + CLEANUP_FREE char *spec = NULL;
> + CLEANUP_FREE char *mp = NULL;
> + CLEANUP_FREE char *mountable = NULL;
> + CLEANUP_FREE char *vfstype = NULL;
> +
> + snprintf (augpath, sizeof augpath, "%s/Mount/What/value", *entry);
> + spec = guestfs_aug_get (g, augpath);
> + if (spec == NULL)
> + return -1;
> +
> + /* Ignore /dev/fd (floppy disks) (RHBZ#642929) and CD-ROM drives.
> + *
> + * /dev/iso9660/FREEBSD_INSTALL can be found in FreeBSDs installation
> + * discs.
> + */
> + if ((STRPREFIX (spec, "/dev/fd") && c_isdigit (spec[7])) ||
> + STREQ (spec, "/dev/floppy") ||
> + STREQ (spec, "/dev/cdrom") ||
> + STRPREFIX (spec, "/dev/iso9660/"))
> + continue;
[etc]
There's a lot of common code with check_fstab here. It's not
completely identical because the augeas queries are slightly
different, but you can parameterize those easily enough.
A nice thing would be a separate commit which first moves the common
code out from check_fstab into its own function.
> +static int
> +check_systemd_mounts (guestfs_h *g, struct inspect_fs *fs)
> +{
> + const char *dirs[] = {"/etc/systemd/", "/lib/systemd/",
> + "/usr/lib/systemd/", NULL};
> + const char **dir;
> + for (dir = dirs; *dir != NULL; dir++) {
> + CLEANUP_FREE_STRING_LIST char **entries = NULL;
> + char **entry;
> +
> + entries = guestfs_find (g, *dir);
Error check? If the command fails it will immediately segfault in the
next line, which is something that inspection code cannot be doing --
it's a DoS attack route.
> + size_t cnt = guestfs_int_count_strings (entries);
> +
> + CLEANUP_FREE_STRING_LIST char **filenames =
> + safe_malloc (g, (cnt + 1) * sizeof (char *));
> +
> + size_t idx = 0;
> + for (entry = entries; *entry != NULL; entry++) {
> + if (STRSUFFIX (*entry, ".mount")) {
> + size_t entry_len = strlen(*dir) + strlen(*entry) + 1;
> + filenames[idx] = safe_malloc (g, entry_len);
> + snprintf (filenames[idx], entry_len, "%s%s", *dir, *entry);
> + ++idx;
> + }
> + }
> + filenames[idx] = NULL;
> +
> + if (inspect_with_augeas2 (g, fs,
> + (const char**) filenames, check_systemd_mnt) == -1) {
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int
> check_fstab (guestfs_h *g, struct inspect_fs *fs)
> {
> CLEANUP_FREE_STRING_LIST char **entries = NULL;
> @@ -1705,6 +1885,66 @@ static char *make_augeas_path_expression (guestfs_h *g, const char **configfiles
> * to 'f' the Augeas handle is closed.
> */
> static int
> +inspect_with_augeas2 (guestfs_h *g, struct inspect_fs *fs,
> + const char **configfiles,
> + int (*f) (guestfs_h *, struct inspect_fs *, const char *))
> +{
As discussed above, I don't believe this function is necessary.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list