On Tue, Jul 19, 2022 at 04:52:04PM -0700, Jeremy Bongio wrote:
> +/*
> + * Use EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID to get/set file system UUID.
> + * Return: 0 on success
> + * -1 when the old method should be used
> + */
> +int handle_fsuuid(__u8 *uuid, bool get)
> +{
> + errcode_t ret;
> + int mnt_flags, fd;
> + char label[FSLABEL_MAX];
> + int maxlen = FSLABEL_MAX - 1;
> + char mntpt[PATH_MAX + 1];
> + struct fsuuid *fsuuid = NULL;
> +
> + fsuuid = malloc(sizeof(*fsuuid) + UUID_SIZE);
> + if (!fsuuid)
> + return -1;
fsuuid is not getting freed in this function, so this will leak
memory.
...
> + if (get)
> + ret = ioctl(fd, EXT4_IOC_GETFSUUID, fsuuid);
> + else
> + ret = ioctl(fd, EXT4_IOC_SETFSUUID, fsuuid);
In the EXT4_IOC_GETFSUUID case, you need to copy fsuuid->fsu_uuid to
uuid, so it is returned to the caller.
> + ret = ext2fs_check_mount_point(device_name, &mnt_flags,
> + mntpt, sizeof(mntpt));
> + if (ret || !(mnt_flags & EXT2_MF_MOUNTED) ||
> + (!get && (mnt_flags & EXT2_MF_READONLY)) ||
> + !mntpt[0])
> + return -1;
handle_fsuuid() is getting called twice by tune2fs when handling the
-U option, which means we're calling ext2fs_check_mount_point() twice.
And around line 3364, tune2fs calls ext2fs_check_if_mounted() which
fetches the mnt_flags but which doesn't get the mountpoint.
So I wonder if wouldn't be better off changing tune2fs's main() to
call ext2fs_check_mount_point() instead of ext2fs_check_if_mounted(),
and we can just determine the mountpoint once.
Then, instead of calling handle_fsuuid/[gs]et_mounted_fsuuid, in the
handling of -U, we can do something like this:
if (U_flag) {
int fd = -1;
struct fsuuid *fsuuid = NULL;
...
if ((mnt_flags & EXT2_MF_MOUNTED) &&
!(mnt_flags & EXT2_MF_READONLY) && mntpt) {
fd = open(mntpt, O_RDONLY);
if (fd >= 0) {
fsuuid = malloc(sizeof(*fsuuid) + UUID_SIZE);
if (!fsuuid) {
close(fd);
fd = -1;
}
}
}
In other words, we can just inline all of handle_fsuuid, and the call
to get_mounted_fsuuid() just becomes:
if (fd >= 0) {
fsuuid->fsu_len - UUID_SIZE;
fsuuid->fsu_flags = 0;
ret = ioctl(fd, EXT4_IOC_GETFSUUID, fsuuid);
}
... and similarly for set_mounted_fsuuid().
Then at the end of tune2fs -U processing, we can do something like this:
if (fd >= 0)
close(fd);
free(fsuuid);
- Ted
