[PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Jeremy Bongio]

This fixes a race between changing the ext4 superblock uuid and operations
like mounting, resizing, changing features, etc.

Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jeremy Bongio <bongiojp@gmail.com>
---

Changes in v4:
Removed unused variable.

 fs/ext4/ext4.h  | 11 +++++++
 fs/ext4/ioctl.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 75b8d81b2469..b760d669a1ca 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -724,6 +724,8 @@ enum {
 #define EXT4_IOC_GETSTATE		_IOW('f', 41, __u32)
 #define EXT4_IOC_GET_ES_CACHE		_IOWR('f', 42, struct fiemap)
 #define EXT4_IOC_CHECKPOINT		_IOW('f', 43, __u32)
+#define EXT4_IOC_GETFSUUID		_IOR('f', 44, struct fsuuid)
+#define EXT4_IOC_SETFSUUID		_IOW('f', 44, struct fsuuid)
 
 #define EXT4_IOC_SHUTDOWN _IOR ('X', 125, __u32)
 
@@ -753,6 +755,15 @@ enum {
 						EXT4_IOC_CHECKPOINT_FLAG_ZEROOUT | \
 						EXT4_IOC_CHECKPOINT_FLAG_DRY_RUN)
 
+/*
+ * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID
+ */
+struct fsuuid {
+	__u32       fsu_len;
+	__u32       fsu_flags;
+	__u8        fsu_uuid[];
+};
+
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
  * ioctl commands in 32 bit emulation
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index cb01c1da0f9d..2c4c3eedfb7b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/iversion.h>
 #include <linux/fileattr.h>
+#include <linux/uuid.h>
 #include "ext4_jbd2.h"
 #include "ext4.h"
 #include <linux/fsmap.h>
@@ -41,6 +42,15 @@ static void ext4_sb_setlabel(struct ext4_super_block *es, const void *arg)
 	memcpy(es->s_volume_name, (char *)arg, EXT4_LABEL_MAX);
 }
 
+/*
+ * Superblock modification callback function for changing file system
+ * UUID.
+ */
+static void ext4_sb_setuuid(struct ext4_super_block *es, const void *arg)
+{
+	memcpy(es->s_uuid, (__u8 *)arg, UUID_SIZE);
+}
+
 static
 int ext4_update_primary_sb(struct super_block *sb, handle_t *handle,
 			   ext4_update_sb_callback func,
@@ -1131,6 +1141,66 @@ static int ext4_ioctl_getlabel(struct ext4_sb_info *sbi, char __user *user_label
 	return 0;
 }
 
+static int ext4_ioctl_getuuid(struct ext4_sb_info *sbi,
+			struct fsuuid __user *ufsuuid)
+{
+	struct fsuuid fsuuid;
+	__u8 uuid[UUID_SIZE];
+
+	if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid)))
+		return -EFAULT;
+
+	if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0)
+		return -EINVAL;
+
+	lock_buffer(sbi->s_sbh);
+	memcpy(uuid, sbi->s_es->s_uuid, UUID_SIZE);
+	unlock_buffer(sbi->s_sbh);
+
+	if (copy_to_user(&ufsuuid->fsu_uuid[0], uuid, UUID_SIZE))
+		return -EFAULT;
+	return 0;
+}
+
+static int ext4_ioctl_setuuid(struct file *filp,
+			const struct fsuuid __user *ufsuuid)
+{
+	int ret = 0;
+	struct super_block *sb = file_inode(filp)->i_sb;
+	struct fsuuid fsuuid;
+	__u8 uuid[UUID_SIZE];
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	/*
+	 * If any checksums (group descriptors or metadata) are being used
+	 * then the checksum seed feature is required to change the UUID.
+	 */
+	if (((ext4_has_feature_gdt_csum(sb) || ext4_has_metadata_csum(sb))
+			&& !ext4_has_feature_csum_seed(sb))
+		|| ext4_has_feature_stable_inodes(sb))
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid)))
+		return -EFAULT;
+
+	if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0)
+		return -EINVAL;
+
+	if (copy_from_user(uuid, &ufsuuid->fsu_uuid[0], UUID_SIZE))
+		return -EFAULT;
+
+	ret = mnt_want_write_file(filp);
+	if (ret)
+		return ret;
+
+	ret = ext4_update_superblocks_fn(sb, ext4_sb_setuuid, &uuid);
+	mnt_drop_write_file(filp);
+
+	return ret;
+}
+
 static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct inode *inode = file_inode(filp);
@@ -1509,6 +1579,10 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return ext4_ioctl_setlabel(filp,
 					   (const void __user *)arg);
 
+	case EXT4_IOC_GETFSUUID:
+		return ext4_ioctl_getuuid(EXT4_SB(sb), (void __user *)arg);
+	case EXT4_IOC_SETFSUUID:
+		return ext4_ioctl_setuuid(filp, (const void __user *)arg);
 	default:
 		return -ENOTTY;
 	}
@@ -1586,6 +1660,8 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case EXT4_IOC_CHECKPOINT:
 	case FS_IOC_GETFSLABEL:
 	case FS_IOC_SETFSLABEL:
+	case EXT4_IOC_GETFSUUID:
+	case EXT4_IOC_SETFSUUID:
 		break;
 	default:
 		return -ENOIOCTLCMD;
-- 
2.37.0.170.g444d1eabd0-goog

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Matthew Wilcox]

On Tue, Jul 19, 2022 at 04:41:31PM -0700, Jeremy Bongio wrote:
> +/*
> + * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID
> + */
> +struct fsuuid {
> +	__u32       fsu_len;
> +	__u32       fsu_flags;
> +	__u8        fsu_uuid[];
> +};

A UUID has a defined size (128 bits):
https://en.wikipedia.org/wiki/Universally_unique_identifier

Why are we defining flags and len?

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 04:18:51AM +0100, Matthew Wilcox wrote:
> On Tue, Jul 19, 2022 at 04:41:31PM -0700, Jeremy Bongio wrote:
> > +/*
> > + * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID
> > + */
> > +struct fsuuid {
> > +	__u32       fsu_len;
> > +	__u32       fsu_flags;
> > +	__u8        fsu_uuid[];
> > +};
> 
> A UUID has a defined size (128 bits):
> https://en.wikipedia.org/wiki/Universally_unique_identifier
> 
> Why are we defining flags and len?

@flags because XFS actually need to add a superblock feature bit
(meta_uuid) to change the UUID while the fs is mounted.  That kind of
change can break backwards compatiblity, so we might want to make
*absolutely sure* that the sysadmin is aware of this:

# xfs_io -c 'setfsuuid 42f3d4d6-d5bb-4e91-a187-2ed0f3c080b2 --to-hell-with-backwards-compatibility' /mnt

@len because some filesystems like vfat have volume identifiers that
aren't actually UUIDs (they're u32); some day someone might want to port
vfat to implement at least the GETFSUUID part (they already have
FAT_IOCTL_GET_VOLUME_ID); and given the amount of confusion that results
when buffer lengths are implied (see [GS]ETFSLABEL) I'd rather this pair
of ioctls be explicit about the buffer length now rather than deal with
the fallout of omitting it now and regretting it later.

--D

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Matthew Wilcox]

On Tue, Jul 19, 2022 at 08:30:57PM -0700, Darrick J. Wong wrote:
> On Wed, Jul 20, 2022 at 04:18:51AM +0100, Matthew Wilcox wrote:
> > On Tue, Jul 19, 2022 at 04:41:31PM -0700, Jeremy Bongio wrote:
> > > +/*
> > > + * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID
> > > + */
> > > +struct fsuuid {
> > > +	__u32       fsu_len;
> > > +	__u32       fsu_flags;
> > > +	__u8        fsu_uuid[];
> > > +};
> > 
> > A UUID has a defined size (128 bits):
> > https://en.wikipedia.org/wiki/Universally_unique_identifier
> > 
> > Why are we defining flags and len?
> 
> @flags because XFS actually need to add a superblock feature bit
> (meta_uuid) to change the UUID while the fs is mounted.  That kind of
> change can break backwards compatiblity, so we might want to make
> *absolutely sure* that the sysadmin is aware of this:

OK.  So we'll define a 'force' flag at some point in the future.  Got it.

> @len because some filesystems like vfat have volume identifiers that
> aren't actually UUIDs (they're u32); some day someone might want to port
> vfat to implement at least the GETFSUUID part (they already have
> FAT_IOCTL_GET_VOLUME_ID); and given the amount of confusion that results
> when buffer lengths are implied (see [GS]ETFSLABEL) I'd rather this pair
> of ioctls be explicit about the buffer length now rather than deal with
> the fallout of omitting it now and regretting it later.

Uhhh.  So what are the semantics of len?  That is, on SET, what does
a filesystem do if userspace says "Here's 8 bytes" but the filesystem
usually uses 16 bytes?  What does the same filesystem do if userspace
offers it 32 bytes?  If the answer is "returns -EINVAL", how does
userspace discover what size of volume ID is acceptable to a particular
filesystem?

And then, on GET, does 'len' just mean "here's the length of the buffer,
put however much will fit into it"?  Should filesystems update it to
inform userspace how much was transferred?

I think there was mention of a manpage earlier.  And if this is truly
for "volume ID" instead of "UUID", then lets call it "volume ID" and
not "UUID" to prevent confusion.

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Theodore Ts'o]

On Tue, Jul 19, 2022 at 08:30:57PM -0700, Darrick J. Wong wrote:
> 
> @len because some filesystems like vfat have volume identifiers that
> aren't actually UUIDs (they're u32)...

It's not just vfat.  Ntfs uses a 64-bit volume identifier, and we
still see both vfat and ntfs on modern-day laptops.  For example, on
my Samsung Galaxy Pro 360, purchased earlier this year and which uses
Secure UEFI boot to dual boot Windows and Debian Linux:

% sudo blkid
/dev/nvme0n1p7: UUID="915eb577-a05d-48ba-ad66-346e14908d19" BLOCK_SIZE="4096" TYPE="ext4" PARTUUID="3194abab-3fe6-4b59-960f-95806d27b1cd"
/dev/nvme0n1p5: LABEL="SAMSUNG_REC" UUID="0A64-BC1B" BLOCK_SIZE="512" TYPE="vfat" PARTLABEL="Basi" PARTUUID="441e92c9-d55b-40ec-4173-636c65706975"
/dev/nvme0n1p3: LABEL="Windows RE tools" BLOCK_SIZE="512" UUID="F49088359087FC7C" TYPE="ntfs" PARTLABEL="M-fM-%M-^WM-fM-^QM-.M-gM-^]M-/M-bM-^AM-3" PARTUUID="0cc7d7ec-6481-40b9-bf23-83b889f020e2"
/dev/nvme0n1p1: LABEL_FATBOOT="SYSTEM" LABEL="SYSTEM" UUID="345B-0F8C" BLOCK_SIZE="512" TYPE="vfat" PARTLABEL="EFI" PARTUUID="13bbf92c-8e02-41fb-93a4-9c4c8328d08a"
/dev/nvme0n1p8: UUID="929a1920-e84c-4797-98b1-2d719e64388f" TYPE="swap" PARTUUID="8d2aad9d-f7a9-47a1-8729-9de367d44696"
/dev/nvme0n1p6: BLOCK_SIZE="512" UUID="82A25B9DA25B950F" TYPE="ntfs" PARTUUID="89bfd94a-3c21-44df-9d6e-c2e66ae1a3ec"
/dev/nvme0n1p4: LABEL="SAMSUNG_REC2" BLOCK_SIZE="512" UUID="A24E62F14E62BDA3" TYPE="ntfs" PARTLABEL="M-fM-^UM-^RM-fM-=M-#M-fM-^UM-6M-gM-%M-2" PARTUUID="9cd299e0-4454-430a-9e96-54ccbf250ff8"

Also note that for better or worse, historically blkid has always
treeated the VFAT and NTFS volume id's as "UUID's", since they serve
the same purpose as UUID's on ext2/ext4/xfs file systems, and so
people may very well have /etc/fstab files which specify a volume by
their UUID:

# /boot/efi was on /dev/nvme0n1p1 during installation
UUID=345B-0F8C  /boot/efi       vfat    umask=0077      0       1

Perhaps a purist would have insisted that we have used "FSVOLID"
instead of "UUID" in blkid almost 20 years ago, in which case perhaps
these ioctl's would have been named FS_IOC_[GS]ETFSVOLID.  But at this
point, it's clearer if we stick with FS_IOC_GETUUID than to try to
introduce change the naming scheme at this point.

					- Ted

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Theodore Ts'o]

On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> Uhhh.  So what are the semantics of len?  That is, on SET, what does
> a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> usually uses 16 bytes?  What does the same filesystem do if userspace
> offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> userspace discover what size of volume ID is acceptable to a particular
> filesystem?
> 
> And then, on GET, does 'len' just mean "here's the length of the buffer,
> put however much will fit into it"?  Should filesystems update it to
> inform userspace how much was transferred?

What I'd suggest is that for GET, the length field when called should
be the length of the buffer, and if the length is too small, we should
return some error --- probably EINVAL or ENOSPC.  If the buffer size
length is larger than what is needed, having the file system update it
with the size of the UUID that was returned.

And this would be how the userspace can discover size of the UUID.  In
practice, though, the human user is going to be suppliyng the UUID,
which means the *human* is going to have to understand that "oh, this
is a VFAT file system, so I need to give 32-bit UUID formatted as
DEAD-BEAF" or "oh, this is a ntfs file system, so I need to enter into
the command line a UUID formatted as the text string
A24E62F14E62BDA3".  (The user might also end up having to ntfs or vfat
specific uuid changing tool; that's unclear at this point.)

As far as Jeremy's patch is concerned, I don't think we need to change
anything forthe SET ioctl, but for the GET util, it would be better in
the extremely unlikely case where the user pass in a length larger
than 16 bytes (say, 32), that we return the 16 byte UUID, and update
the length field to be 16 bytes.

I don't think it's strictly necessary, since in practice there is no
reason why a file system's volume identifier would ever be larger than
16 bytes --- the chances that we might need an extra 240 bytes to
specify a multiverse identifier seems.... unlikely.  :-)

				- Ted

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Matthew Wilcox]

On Wed, Jul 20, 2022 at 02:00:25PM -0400, Theodore Ts'o wrote:
> On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> > Uhhh.  So what are the semantics of len?  That is, on SET, what does
> > a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> > usually uses 16 bytes?  What does the same filesystem do if userspace
> > offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> > userspace discover what size of volume ID is acceptable to a particular
> > filesystem?
> > 
> > And then, on GET, does 'len' just mean "here's the length of the buffer,
> > put however much will fit into it"?  Should filesystems update it to
> > inform userspace how much was transferred?
> 
> What I'd suggest is that for GET, the length field when called should
> be the length of the buffer, and if the length is too small, we should
> return some error --- probably EINVAL or ENOSPC.  If the buffer size
> length is larger than what is needed, having the file system update it
> with the size of the UUID that was returned.
> 
> And this would be how the userspace can discover size of the UUID.  In
> practice, though, the human user is going to be suppliyng the UUID,
> which means the *human* is going to have to understand that "oh, this
> is a VFAT file system, so I need to give 32-bit UUID formatted as
> DEAD-BEAF" or "oh, this is a ntfs file system, so I need to enter into
> the command line a UUID formatted as the text string
> A24E62F14E62BDA3".  (The user might also end up having to ntfs or vfat
> specific uuid changing tool; that's unclear at this point.)

I think you covered all my questions there except for what happens
if the user tried to set ext4 to 0xDEADBEEF; that should return
-EINVAL?  They could specify 0xDEADBEEF'00000000'00000000'00000000 or
0x00000000'00000000'00000000'DEADBEEF, but it'd be up to them to choose
which one they wanted rather than have the filesystem pad it out for them?

> As far as Jeremy's patch is concerned, I don't think we need to change
> anything forthe SET ioctl, but for the GET util, it would be better in
> the extremely unlikely case where the user pass in a length larger
> than 16 bytes (say, 32), that we return the 16 byte UUID, and update
> the length field to be 16 bytes.
> 
> I don't think it's strictly necessary, since in practice there is no
> reason why a file system's volume identifier would ever be larger than
> 16 bytes --- the chances that we might need an extra 240 bytes to
> specify a multiverse identifier seems.... unlikely.  :-)

Yes, 128 bits is sufficiently unique for this use case.

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 07:27:02PM +0100, Matthew Wilcox wrote:
> On Wed, Jul 20, 2022 at 02:00:25PM -0400, Theodore Ts'o wrote:
> > On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> > > Uhhh.  So what are the semantics of len?  That is, on SET, what does
> > > a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> > > usually uses 16 bytes?  What does the same filesystem do if userspace
> > > offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> > > userspace discover what size of volume ID is acceptable to a particular
> > > filesystem?
> > > 
> > > And then, on GET, does 'len' just mean "here's the length of the buffer,
> > > put however much will fit into it"?  Should filesystems update it to
> > > inform userspace how much was transferred?
> > 
> > What I'd suggest is that for GET, the length field when called should
> > be the length of the buffer, and if the length is too small, we should
> > return some error --- probably EINVAL or ENOSPC.  If the buffer size
> > length is larger than what is needed, having the file system update it
> > with the size of the UUID that was returned.

I'd suggest something different -- calling the getfsuuid ioctl with a
null argument should return the filesystem's volid/uuid size as the
return value.  If userspace supplies a non-null argument, then fsu_len
has to match the filesystem's volid/uuid size or else you get EINVAL.

--D

> > And this would be how the userspace can discover size of the UUID.  In
> > practice, though, the human user is going to be suppliyng the UUID,
> > which means the *human* is going to have to understand that "oh, this
> > is a VFAT file system, so I need to give 32-bit UUID formatted as
> > DEAD-BEAF" or "oh, this is a ntfs file system, so I need to enter into
> > the command line a UUID formatted as the text string
> > A24E62F14E62BDA3".  (The user might also end up having to ntfs or vfat
> > specific uuid changing tool; that's unclear at this point.)
> 
> I think you covered all my questions there except for what happens
> if the user tried to set ext4 to 0xDEADBEEF; that should return
> -EINVAL?  They could specify 0xDEADBEEF'00000000'00000000'00000000 or
> 0x00000000'00000000'00000000'DEADBEEF, but it'd be up to them to choose
> which one they wanted rather than have the filesystem pad it out for them?
> 
> > As far as Jeremy's patch is concerned, I don't think we need to change
> > anything forthe SET ioctl, but for the GET util, it would be better in
> > the extremely unlikely case where the user pass in a length larger
> > than 16 bytes (say, 32), that we return the 16 byte UUID, and update
> > the length field to be 16 bytes.
> > 
> > I don't think it's strictly necessary, since in practice there is no
> > reason why a file system's volume identifier would ever be larger than
> > 16 bytes --- the chances that we might need an extra 240 bytes to
> > specify a multiverse identifier seems.... unlikely.  :-)
> 
> Yes, 128 bits is sufficiently unique for this use case.

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Matthew Wilcox]

On Wed, Jul 20, 2022 at 11:47:08AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 20, 2022 at 07:27:02PM +0100, Matthew Wilcox wrote:
> > On Wed, Jul 20, 2022 at 02:00:25PM -0400, Theodore Ts'o wrote:
> > > On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> > > > Uhhh.  So what are the semantics of len?  That is, on SET, what does
> > > > a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> > > > usually uses 16 bytes?  What does the same filesystem do if userspace
> > > > offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> > > > userspace discover what size of volume ID is acceptable to a particular
> > > > filesystem?
> > > > 
> > > > And then, on GET, does 'len' just mean "here's the length of the buffer,
> > > > put however much will fit into it"?  Should filesystems update it to
> > > > inform userspace how much was transferred?
> > > 
> > > What I'd suggest is that for GET, the length field when called should
> > > be the length of the buffer, and if the length is too small, we should
> > > return some error --- probably EINVAL or ENOSPC.  If the buffer size
> > > length is larger than what is needed, having the file system update it
> > > with the size of the UUID that was returned.
> 
> I'd suggest something different -- calling the getfsuuid ioctl with a
> null argument should return the filesystem's volid/uuid size as the
> return value.  If userspace supplies a non-null argument, then fsu_len
> has to match the filesystem's volid/uuid size or else you get EINVAL.

Or userspace passes in 0 for the len and the filesystem returns -EINVAL
and sets ->len to what the valid size would be?  There's a few ways of
solving this.

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Jeremy Bongio]

On Wed, Jul 20, 2022 at 12:09 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jul 20, 2022 at 11:47:08AM -0700, Darrick J. Wong wrote:
> > On Wed, Jul 20, 2022 at 07:27:02PM +0100, Matthew Wilcox wrote:
> > > On Wed, Jul 20, 2022 at 02:00:25PM -0400, Theodore Ts'o wrote:
> > > > On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> > > > > Uhhh.  So what are the semantics of len?  That is, on SET, what does
> > > > > a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> > > > > usually uses 16 bytes?  What does the same filesystem do if userspace
> > > > > offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> > > > > userspace discover what size of volume ID is acceptable to a particular
> > > > > filesystem?
> > > > >
> > > > > And then, on GET, does 'len' just mean "here's the length of the buffer,
> > > > > put however much will fit into it"?  Should filesystems update it to
> > > > > inform userspace how much was transferred?
> > > >
> > > > What I'd suggest is that for GET, the length field when called should
> > > > be the length of the buffer, and if the length is too small, we should
> > > > return some error --- probably EINVAL or ENOSPC.  If the buffer size
> > > > length is larger than what is needed, having the file system update it
> > > > with the size of the UUID that was returned.
> >
> > I'd suggest something different -- calling the getfsuuid ioctl with a
> > null argument should return the filesystem's volid/uuid size as the
> > return value.  If userspace supplies a non-null argument, then fsu_len
> > has to match the filesystem's volid/uuid size or else you get EINVAL.
>
> Or userspace passes in 0 for the len and the filesystem returns -EINVAL
> and sets ->len to what the valid size would be?  There's a few ways of
> solving this.

This solution seems more intuitive to me. If EXT4_IOCTL_GETFSUUID is
called with fsu_len set to 0, then fsu_len will be set to the required
UUID length and return with an error code.

I discussed this solution when first developing the ioctl, but I left
it out since for ext4 I don't have a use case. However since other
filesystems will likely implement this ioctl, it makes sense to add.

I'll send out a new manpage with that detail added and update the code.

Re: [PATCH v4] Add ioctls to get/set the ext4 superblock uuid.

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 01:11:25PM -0700, Jeremy Bongio wrote:
> On Wed, Jul 20, 2022 at 12:09 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Jul 20, 2022 at 11:47:08AM -0700, Darrick J. Wong wrote:
> > > On Wed, Jul 20, 2022 at 07:27:02PM +0100, Matthew Wilcox wrote:
> > > > On Wed, Jul 20, 2022 at 02:00:25PM -0400, Theodore Ts'o wrote:
> > > > > On Wed, Jul 20, 2022 at 03:11:21PM +0100, Matthew Wilcox wrote:
> > > > > > Uhhh.  So what are the semantics of len?  That is, on SET, what does
> > > > > > a filesystem do if userspace says "Here's 8 bytes" but the filesystem
> > > > > > usually uses 16 bytes?  What does the same filesystem do if userspace
> > > > > > offers it 32 bytes?  If the answer is "returns -EINVAL", how does
> > > > > > userspace discover what size of volume ID is acceptable to a particular
> > > > > > filesystem?
> > > > > >
> > > > > > And then, on GET, does 'len' just mean "here's the length of the buffer,
> > > > > > put however much will fit into it"?  Should filesystems update it to
> > > > > > inform userspace how much was transferred?
> > > > >
> > > > > What I'd suggest is that for GET, the length field when called should
> > > > > be the length of the buffer, and if the length is too small, we should
> > > > > return some error --- probably EINVAL or ENOSPC.  If the buffer size
> > > > > length is larger than what is needed, having the file system update it
> > > > > with the size of the UUID that was returned.
> > >
> > > I'd suggest something different -- calling the getfsuuid ioctl with a
> > > null argument should return the filesystem's volid/uuid size as the
> > > return value.  If userspace supplies a non-null argument, then fsu_len
> > > has to match the filesystem's volid/uuid size or else you get EINVAL.
> >
> > Or userspace passes in 0 for the len and the filesystem returns -EINVAL
> > and sets ->len to what the valid size would be?  There's a few ways of
> > solving this.
> 
> This solution seems more intuitive to me. If EXT4_IOCTL_GETFSUUID is
> called with fsu_len set to 0, then fsu_len will be set to the required
> UUID length and return with an error code.

Works for me!

> I discussed this solution when first developing the ioctl, but I left
> it out since for ext4 I don't have a use case. However since other
> filesystems will likely implement this ioctl, it makes sense to add.

Hee hee, future thinking.  That's what a good ARB should be for <cough>.

> I'll send out a new manpage with that detail added and update the code.

I'll look forward to it. :)

--D