On Wed, Nov 09, 2022 at 02:23:35PM -0800, Catherine Hoang wrote:
> Add support for the fsuuid command to retrieve the UUID of a mounted
> filesystem.
>
> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
> ---
> spaceman/Makefile | 4 +--
> spaceman/fsuuid.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
> spaceman/init.c | 1 +
> spaceman/space.h | 1 +
> 4 files changed, 67 insertions(+), 2 deletions(-)
> create mode 100644 spaceman/fsuuid.c
>
> diff --git a/spaceman/Makefile b/spaceman/Makefile
> index 1f048d54..901e4e6d 100644
> --- a/spaceman/Makefile
> +++ b/spaceman/Makefile
> @@ -7,10 +7,10 @@ include $(TOPDIR)/include/builddefs
>
> LTCOMMAND = xfs_spaceman
> HFILES = init.h space.h
> -CFILES = info.c init.c file.c health.c prealloc.c trim.c
> +CFILES = info.c init.c file.c health.c prealloc.c trim.c fsuuid.c
> LSRCFILES = xfs_info.sh
>
> -LLDLIBS = $(LIBXCMD) $(LIBFROG)
> +LLDLIBS = $(LIBXCMD) $(LIBFROG) $(LIBUUID)
> LTDEPENDENCIES = $(LIBXCMD) $(LIBFROG)
> LLDFLAGS = -static
>
> diff --git a/spaceman/fsuuid.c b/spaceman/fsuuid.c
> new file mode 100644
> index 00000000..be12c1ad
> --- /dev/null
> +++ b/spaceman/fsuuid.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Oracle.
> + * All Rights Reserved.
> + */
> +
> +#include "libxfs.h"
> +#include "libfrog/fsgeom.h"
> +#include "libfrog/paths.h"
> +#include "command.h"
> +#include "init.h"
> +#include "space.h"
> +#include <sys/ioctl.h>
> +
> +#ifndef FS_IOC_GETFSUUID
> +#define FS_IOC_GETFSUUID _IOR('f', 44, struct fsuuid)
> +#define UUID_SIZE 16
> +struct fsuuid {
> + __u32 fsu_len;
> + __u32 fsu_flags;
> + __u8 fsu_uuid[];
This is a flex array ^^ which has no size. struct fsuuid therefore
has a size of 8 bytes (i.e. enough to cover the two u32 fields) and no
more. It's assumed that the caller will allocate the memory for
fsu_uuid...
> +};
> +#endif
> +
> +static cmdinfo_t fsuuid_cmd;
> +
> +static int
> +fsuuid_f(
> + int argc,
> + char **argv)
> +{
> + struct fsuuid fsuuid;
> + int error;
...which makes this usage a problem, because we've not reserved any
space on the stack to hold the UUID. The kernel will blindly assume
that there are fsuuid.fsu_len bytes after fsuuid and write to them,
which will clobber something on the stack.
If you're really unlucky, the C compiler will put the fsuuid right
before the call frame, which is how stack smashing attacks work. It
might also lay out bp[] immediately afterwards, which will give you
weird results as the unparse function overwrites its source buffer. The
C compiler controls the stack layout, which means this can go bad in
subtle ways.
Either way, gcc complains about this (albeit in an opaque manner)...
In file included from ../include/xfs.h:9,
from ../include/libxfs.h:15,
from fsuuid.c:7:
In function ‘platform_uuid_unparse’,
inlined from ‘fsuuid_f’ at fsuuid.c:45:3:
../include/xfs/linux.h:100:9: error: ‘uuid_unparse’ reading 16 bytes from a region of size 0 [-Werror=stringop-overread]
100 | uuid_unparse(*uu, buffer);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
../include/xfs/linux.h: In function ‘fsuuid_f’:
../include/xfs/linux.h:100:9: note: referencing argument 1 of type ‘const unsigned char *’
In file included from ../include/xfs/linux.h:13,
from ../include/xfs.h:9,
from ../include/libxfs.h:15,
from fsuuid.c:7:
/usr/include/uuid/uuid.h:107:13: note: in a call to function ‘uuid_unparse’
107 | extern void uuid_unparse(const uuid_t uu, char *out);
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
...so please allocate the struct fsuuid object dynamically.
--D
> + char bp[40];
> +
> + fsuuid.fsu_len = UUID_SIZE;
> + fsuuid.fsu_flags = 0;
> +
> + error = ioctl(file->xfd.fd, FS_IOC_GETFSUUID, &fsuuid);
> +
> + if (error) {
> + perror("fsuuid");
> + exitcode = 1;
> + } else {
> + platform_uuid_unparse((uuid_t *)fsuuid.fsu_uuid, bp);
> + printf("UUID = %s\n", bp);
> + }
> +
> + return 0;
> +}
> +
> +void
> +fsuuid_init(void)
> +{
> + fsuuid_cmd.name = "fsuuid";
> + fsuuid_cmd.cfunc = fsuuid_f;
> + fsuuid_cmd.argmin = 0;
> + fsuuid_cmd.argmax = 0;
> + fsuuid_cmd.flags = CMD_FLAG_ONESHOT;
> + fsuuid_cmd.oneline = _("get mounted filesystem UUID");
> +
> + add_command(&fsuuid_cmd);
> +}
> diff --git a/spaceman/init.c b/spaceman/init.c
> index cf1ff3cb..efe1bf9b 100644
> --- a/spaceman/init.c
> +++ b/spaceman/init.c
> @@ -35,6 +35,7 @@ init_commands(void)
> trim_init();
> freesp_init();
> health_init();
> + fsuuid_init();
> }
>
> static int
> diff --git a/spaceman/space.h b/spaceman/space.h
> index 723209ed..dcbdca08 100644
> --- a/spaceman/space.h
> +++ b/spaceman/space.h
> @@ -33,5 +33,6 @@ extern void freesp_init(void);
> #endif
> extern void info_init(void);
> extern void health_init(void);
> +extern void fsuuid_init(void);
>
> #endif /* XFS_SPACEMAN_SPACE_H_ */
> --
> 2.25.1
>
