f2fs: reserve bits for fs-verity

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

[1] https://marc.info/?l=linux-fsdevel&m=151690752225644
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mhalcrow/linux.git/log/?h=fs-verity-dev

Signed-off-by Eric Biggers <[email protected]>

---
 fs/f2fs/f2fs.h | 2 ++
 1 file changed, 2 insertions(+)

fs/f2fs/f2fs.h +2 -0

index ae69dc358401..96d7809c4541 100644
@@ -146,6 +146,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_QUOTA_INO		0x0080
 #define F2FS_FEATURE_INODE_CRTIME	0x0100
 #define F2FS_FEATURE_LOST_FOUND		0x0200
+#define F2FS_FEATURE_VERITY		0x0400	/* reserved */
 
 #define F2FS_HAS_FEATURE(sb, mask)					\
 	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)

@@ -598,6 +599,7 @@ enum {
 #define FADVISE_ENC_NAME_BIT	0x08
 #define FADVISE_KEEP_SIZE_BIT	0x10
 #define FADVISE_HOT_BIT		0x20
+#define FADVISE_VERITY_BIT	0x40	/* reserved */
 
 #define file_is_cold(inode)	is_file(inode, FADVISE_COLD_BIT)
 #define file_wrong_pino(inode)	is_file(inode, FADVISE_LOST_PINO_BIT)

Hi Eric,

Quoted 9 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Quoted 6 lines

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

And how about applying this patch inside the patchset of new fsverity feature? Since once fsverity feature has some design modification, I worry about that may be we need to change this bit? result in disk layout incompatibility.

Thanks,

Quoted 30 lines

[1] https://marc.info/?l=linux-fsdevel&m=151690752225644
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mhalcrow/linux.git/log/?h=fs-verity-dev

Signed-off-by Eric Biggers <[email protected]>

---
 fs/f2fs/f2fs.h | 2 ++
 1 file changed, 2 insertions(+)

fs/f2fs/f2fs.h +2 -0

index ae69dc358401..96d7809c4541 100644
@@ -146,6 +146,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_QUOTA_INO		0x0080
 #define F2FS_FEATURE_INODE_CRTIME	0x0100
 #define F2FS_FEATURE_LOST_FOUND		0x0200
+#define F2FS_FEATURE_VERITY		0x0400	/* reserved */
 
 #define F2FS_HAS_FEATURE(sb, mask)					\
 	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)

@@ -598,6 +599,7 @@ enum {
 #define FADVISE_ENC_NAME_BIT	0x08
 #define FADVISE_KEEP_SIZE_BIT	0x10
 #define FADVISE_HOT_BIT		0x20
+#define FADVISE_VERITY_BIT	0x40	/* reserved */
 
 #define file_is_cold(inode)	is_file(inode, FADVISE_COLD_BIT)
 #define file_wrong_pino(inode)	is_file(inode, FADVISE_LOST_PINO_BIT)

Quoted 19 lines · Chao Yu

Hi Eric,

Quoted 15 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

Quoted 3 lines

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

Do we need to get/set this bit of i_flags to user? And, f2fs doesn't synchronize it with inode block update. I think this should be set by internal f2fs operations likewise fscrypt.

Quoted 4 lines

And how about applying this patch inside the patchset of new fsverity feature? Since once fsverity feature has some design modification, I worry about that may be we need to change this bit? result in disk layout incompatibility.

The whole body is not fully mergeable, so once reserving the bits first, we can support it f2fs-tools and prepare the feature in advance. Based on previous fscrypt experience, I don't expect we need to modify or drop these dramatically later.

Any thoughts?

Hi Chao and Jaegeuk,

Quoted 24 lines · Jaegeuk Kim

Quoted 22 lines · Chao Yu

Hi Eric,

Quoted 9 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Quoted 6 lines

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

Quoted 4 lines

Do we need to get/set this bit of i_flags to user? And, f2fs doesn't synchronize it with inode block update. I think this should be set by internal f2fs operations likewise fscrypt.

The fs-verity inode flag won't be modifiable using FS_IOC_SETFLAGS. Like

fscrypt, it will only be possible to set it using a dedicated ioctl (tentatively called FS_IOC_ENABLE_VERITY), and it won't be supported to clear the bit once set, short of deleting and re-creating the file. So it doesn't really matter where the bit goes in the on-disk inode, it just needs to go somewhere. I'm just hesitant to reserve a flag in the UAPI flags namespace which is really more "owned" by ext4 than by f2fs, so has more implications than just f2fs as we would effectively be reserving the flag for ext4's on-disk format too.

I do think the flag *should* go in i_flags rather than i_advise, but I think the assumption that f2fs's inode flags namespace matches ext4's would first need to be removed so as to not tie the on-disk formats of different filesystems together.

Quoted 11 lines

Quoted 4 lines

And how about applying this patch inside the patchset of new fsverity feature? Since once fsverity feature has some design modification, I worry about that may be we need to change this bit? result in disk layout incompatibility.

The whole body is not fully mergeable, so once reserving the bits first, we can support it f2fs-tools and prepare the feature in advance. Based on previous fscrypt experience, I don't expect we need to modify or drop these dramatically later.

Any thoughts?

Quoted 1 line

Yep, the full patchset isn't ready to be merged upstream yet. Other parts of the API/ABI such as the fs-verity metadata format, the ioctl numbers, and the semantics of accessing such files is subject to change. We know we'll need a superblock feature flag and a per-inode bit in any case, though. Personally I'd prefer to wait for the full patchset too, but we have people who want to start using the prototype of the feature already, so having f2fs-tools support the feature flag and having the bits not conflict with other f2fs features will be helpful.

Thanks,

Eric

.

Hi Eric and Jaegeuk,

Quoted 27 lines · Eric Biggers

Hi Chao and Jaegeuk,

Quoted 24 lines · Jaegeuk Kim

Quoted 22 lines · Chao Yu

Hi Eric,

Quoted 9 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Quoted 6 lines

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

Quoted 6 lines

Quoted 4 lines

Do we need to get/set this bit of i_flags to user? And, f2fs doesn't synchronize it with inode block update. I think this should be set by internal f2fs operations likewise fscrypt.

The fs-verity inode flag won't be modifiable using FS_IOC_SETFLAGS. Like

Verity flag can also be wrapped by FS_FL_USER_MODIFIABLE like for FS_ENCRYPT_FL?

Quoted 7 lines

fscrypt, it will only be possible to set it using a dedicated ioctl (tentatively called FS_IOC_ENABLE_VERITY), and it won't be supported to clear the bit once set, short of deleting and re-creating the file. So it doesn't really matter where the bit goes in the on-disk inode, it just needs to go somewhere. I'm just hesitant to reserve a flag in the UAPI flags namespace which is really more "owned" by ext4 than by f2fs, so has more implications than just f2fs as we would effectively be reserving the flag for ext4's on-disk format too.

IMO, because this is a VFS feature, it will be better that we can put it in more generic place, also user can check this bit in generic way (via FS_IOC_GETFLAGS), and then for other filesystem who wants add this feature, that will be simple to place this bit.

What I can see is, for encryption feature:

vfs::i_flags
#define FS_ENCRYPT_FL 0x00000800 /* Encrypted file */

ext4:i_flags
#define EXT4_ENCRYPT_FL 0x00000800 /* encrypted file */

f2fs::i_advice
#define FADVISE_ENCRYPT_BIT 0x04

It's very wired that f2fs didn't use well defined FS_ENCRYPT_FL bit position, result in that we leave a hole in on-disk i_flags, and if we want to show the same 'encrypted' flags status in FS_IOC_GETFLAGS, we need to change more codes.

Anyway, I just ask why not let generic status goes into i_flags, and private status goes into i_advices?

Quoted 18 lines

I do think the flag *should* go in i_flags rather than i_advise, but I think the assumption that f2fs's inode flags namespace matches ext4's would first need to be removed so as to not tie the on-disk formats of different filesystems together.

Quoted 11 lines

Quoted 4 lines

And how about applying this patch inside the patchset of new fsverity feature? Since once fsverity feature has some design modification, I worry about that may be we need to change this bit? result in disk layout incompatibility.

The whole body is not fully mergeable, so once reserving the bits first, we can support it f2fs-tools and prepare the feature in advance. Based on previous fscrypt experience, I don't expect we need to modify or drop these dramatically later.

Any thoughts?

Since I don't know current progress of this feature development, I hope this feature will not be against by vfs developers or suffer design change after we reserved bit for it. :)

Quoted 10 lines

Quoted 1 line

Yep, the full patchset isn't ready to be merged upstream yet. Other parts of the API/ABI such as the fs-verity metadata format, the ioctl numbers, and the semantics of accessing such files is subject to change. We know we'll need a superblock feature flag and a per-inode bit in any case, though. Personally I'd prefer to wait for the full patchset too, but we have people who want to start using the prototype of the feature already, so having f2fs-tools support the feature flag and having the bits not conflict with other f2fs features will be helpful.

Oh, so we want a stable on-disk layout, so image for experience will contain fsverity bit in stable position, after formal android released, image with fsverity feature can still be valid, right?

[+Cc linux-ext4, linux-fsdevel]

Quoted 63 lines · Chao Yu

Hi Eric and Jaegeuk,

Quoted 27 lines · Eric Biggers

Hi Chao and Jaegeuk,

Quoted 24 lines · Jaegeuk Kim

Quoted 22 lines · Chao Yu

Hi Eric,

Quoted 9 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Quoted 6 lines

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

Quoted 6 lines

Quoted 4 lines

Do we need to get/set this bit of i_flags to user? And, f2fs doesn't synchronize it with inode block update. I think this should be set by internal f2fs operations likewise fscrypt.

The fs-verity inode flag won't be modifiable using FS_IOC_SETFLAGS. Like

Verity flag can also be wrapped by FS_FL_USER_MODIFIABLE like for FS_ENCRYPT_FL?

Quoted 7 lines

fscrypt, it will only be possible to set it using a dedicated ioctl (tentatively called FS_IOC_ENABLE_VERITY), and it won't be supported to clear the bit once set, short of deleting and re-creating the file. So it doesn't really matter where the bit goes in the on-disk inode, it just needs to go somewhere. I'm just hesitant to reserve a flag in the UAPI flags namespace which is really more "owned" by ext4 than by f2fs, so has more implications than just f2fs as we would effectively be reserving the flag for ext4's on-disk format too.

IMO, because this is a VFS feature, it will be better that we can put it in more generic place, also user can check this bit in generic way (via FS_IOC_GETFLAGS), and then for other filesystem who wants add this feature, that will be simple to place this bit.

What I can see is, for encryption feature:

vfs::i_flags
#define FS_ENCRYPT_FL 0x00000800 /* Encrypted file */

ext4:i_flags
#define EXT4_ENCRYPT_FL 0x00000800 /* encrypted file */

f2fs::i_advice
#define FADVISE_ENCRYPT_BIT 0x04

Quoted 6 lines

It's very wired that f2fs didn't use well defined FS_ENCRYPT_FL bit position, result in that we leave a hole in on-disk i_flags, and if we want to show the same 'encrypted' flags status in FS_IOC_GETFLAGS, we need to change more codes.

Anyway, I just ask why not let generic status goes into i_flags, and private status goes into i_advices?

Ted and others, what would you say about allocating FS_VERITY_FL as one of the unused ext4 / "VFS" inode flags like 0x01000000, or maybe 0x00000400 if the compression flags aren't being used anymore?

I do wish that we separated the on-disk flags namespaces from the FS_IOC_GETFLAGS/FS_IOC_SETFLAGS namespace though... Then adding the flag to

Quoted 73 lines · Eric Biggers

[+Cc linux-ext4, linux-fsdevel]

Quoted 69 lines · Chao Yu

Hi Eric and Jaegeuk,

Quoted 27 lines · Eric Biggers

Hi Chao and Jaegeuk,

Quoted 24 lines · Jaegeuk Kim

Quoted 22 lines · Chao Yu

Hi Eric,

Quoted 9 lines · Eric Biggers

Reserve an F2FS feature flag and inode flag for fs-verity. This is an in-development feature that is planned be discussed at LSF/MM 2018 [1]. It will provide file-based integrity and authenticity for read-only files. Most code will be in a filesystem-independent module, with smaller changes needed to individual filesystems that opt-in to supporting the feature. An early prototype supporting F2FS is available [2]. Reserving the F2FS on-disk bits for fs-verity will prevent users of the prototype from conflicting with other new F2FS features.

Quoted 6 lines

Note that we're reserving the inode flag in f2fs_inode.i_advise, which isn't really appropriate since it's not a hint or advice. But ->i_advise is already being used to hold the 'encrypt' flag; and F2FS's ->i_flags uses the generic FS_* values, so it seems ->i_flags can't be used for an F2FS-specific flag without additional work to remove the assumption that ->i_flags uses the generic flags namespace.

At a glance, this is a VFS feature, can we search free slot, and define FS_VERITY_FL like other generic flags, so we can intergrate this flag into f2fs_inode::i_flags?

Quoted 6 lines

Quoted 4 lines

Do we need to get/set this bit of i_flags to user? And, f2fs doesn't synchronize it with inode block update. I think this should be set by internal f2fs operations likewise fscrypt.

The fs-verity inode flag won't be modifiable using FS_IOC_SETFLAGS. Like

Verity flag can also be wrapped by FS_FL_USER_MODIFIABLE like for FS_ENCRYPT_FL?

Quoted 7 lines

fscrypt, it will only be possible to set it using a dedicated ioctl (tentatively called FS_IOC_ENABLE_VERITY), and it won't be supported to clear the bit once set, short of deleting and re-creating the file. So it doesn't really matter where the bit goes in the on-disk inode, it just needs to go somewhere. I'm just hesitant to reserve a flag in the UAPI flags namespace which is really more "owned" by ext4 than by f2fs, so has more implications than just f2fs as we would effectively be reserving the flag for ext4's on-disk format too.

IMO, because this is a VFS feature, it will be better that we can put it in more generic place, also user can check this bit in generic way (via FS_IOC_GETFLAGS), and then for other filesystem who wants add this feature, that will be simple to place this bit.

What I can see is, for encryption feature:

vfs::i_flags
#define FS_ENCRYPT_FL 0x00000800 /* Encrypted file */

ext4:i_flags
#define EXT4_ENCRYPT_FL 0x00000800 /* encrypted file */

f2fs::i_advice
#define FADVISE_ENCRYPT_BIT 0x04

It's very wired that f2fs didn't use well defined FS_ENCRYPT_FL bit position, result in that we leave a hole in on-disk i_flags, and if we want to show the same 'encrypted' flags status in FS_IOC_GETFLAGS, we need to change more codes.

Anyway, I just ask why not let generic status goes into i_flags, and private status goes into i_advices?

Quoted 6 lines

Ted and others, what would you say about allocating FS_VERITY_FL as one of the unused ext4 / "VFS" inode flags like 0x01000000, or maybe 0x00000400 if the compression flags aren't being used anymore?

I do wish that we separated the on-disk flags namespaces from the FS_IOC_GETFLAGS/FS_IOC_SETFLAGS namespace though... Then adding the flag to

Use FS_IOC_{GS}ETXATTR instead?. I'ts not tied to an on-disk format.