| Message ID | 20240817093256.222226-1-cem@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [V2] xfs_io: Fix fscrypt macros ordering | expand |
Hi Carlos, cem@kernel.org wrote, > From: Carlos Maiolino <cem@kernel.org> > > We've been reported a failure to build xfsprogs within buildroot's > environment when they tried to upgrade xfsprogs from 6.4 to 6.9: > > encrypt.c:53:36: error: 'FSCRYPT_KEY_IDENTIFIER_SIZE' undeclared > here (not in a function) > 53 | __u8 > master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > encrypt.c:61:42: error: field 'v1' has incomplete type > 61 | struct fscrypt_policy_v1 v1; > | ^~ > > They were using a kernel version without FS_IOC_GET_ENCRYPTION_POLICY_EX > set and OVERRIDE_SYSTEM_FSCRYPT_POLICY_V2 was unset. > This combination caused xfsprogs to attempt to define fscrypt_policy_v2 > locally, which uses: > __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; > > The problem is FSCRYPT_KEY_IDENTIFIER_SIZE is only after this block of > code, so we need to define it earlier. > > This also attempts to use fscrypt_policy_v1, which is defined only > later. > > To fix this, just reorder both ifdef blocks, but we need to move the > definition of FS_IOC_GET_ENCRYPTION_POLICY_EX to the later, otherwise, > the later definitions won't be enabled causing havoc. > > Fixes: e97caf714697a ("xfs_io/encrypt: support specifying crypto data unit size") > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > V2: > - Remove dangling leftover comment > - define FS_IOC_GET_ENCRYPTION_POLICY_EX on it's own block. Thanks for the V2. I tested it on the failing toolchains and it works now. You can add a Tested-by: Waldemar Brodkorb <wbx@openadk.org> best regards Waldemar
On Sat, Aug 17, 2024 at 11:32:48AM +0200, cem@kernel.org wrote: > From: Carlos Maiolino <cem@kernel.org> > > We've been reported a failure to build xfsprogs within buildroot's > environment when they tried to upgrade xfsprogs from 6.4 to 6.9: > > encrypt.c:53:36: error: 'FSCRYPT_KEY_IDENTIFIER_SIZE' undeclared > here (not in a function) > 53 | __u8 > master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > encrypt.c:61:42: error: field 'v1' has incomplete type > 61 | struct fscrypt_policy_v1 v1; > | ^~ > > They were using a kernel version without FS_IOC_GET_ENCRYPTION_POLICY_EX > set and OVERRIDE_SYSTEM_FSCRYPT_POLICY_V2 was unset. > This combination caused xfsprogs to attempt to define fscrypt_policy_v2 > locally, which uses: > __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; > > The problem is FSCRYPT_KEY_IDENTIFIER_SIZE is only after this block of > code, so we need to define it earlier. > > This also attempts to use fscrypt_policy_v1, which is defined only > later. > > To fix this, just reorder both ifdef blocks, but we need to move the > definition of FS_IOC_GET_ENCRYPTION_POLICY_EX to the later, otherwise, > the later definitions won't be enabled causing havoc. > > Fixes: e97caf714697a ("xfs_io/encrypt: support specifying crypto data unit size") > Reported-by: Waldemar Brodkorb <wbx@openadk.org> > Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > V2: > - Remove dangling leftover comment > - define FS_IOC_GET_ENCRYPTION_POLICY_EX on it's own block. > > Bill, as the updates for the V2 are trivial, I'm keeping your RwB, > hopefuly you agree :) > > io/encrypt.c | 67 ++++++++++++++++++++++++++-------------------------- > 1 file changed, 34 insertions(+), 33 deletions(-) Reviewed-by: Eric Biggers <ebiggers@google.com> - Eric
diff --git a/io/encrypt.c b/io/encrypt.c index 79061b07c..333ec03df 100644 --- a/io/encrypt.c +++ b/io/encrypt.c @@ -35,35 +35,6 @@ #define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy) #endif -/* - * Since the log2_data_unit_size field was added later than fscrypt_policy_v2 - * itself, we may need to override the system definition to get that field. - * And also fscrypt_get_policy_ex_arg since it contains fscrypt_policy_v2. - */ -#if !defined(FS_IOC_GET_ENCRYPTION_POLICY_EX) || \ - defined(OVERRIDE_SYSTEM_FSCRYPT_POLICY_V2) -#undef fscrypt_policy_v2 -struct fscrypt_policy_v2 { - __u8 version; - __u8 contents_encryption_mode; - __u8 filenames_encryption_mode; - __u8 flags; - __u8 log2_data_unit_size; - __u8 __reserved[3]; - __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; -}; - -#undef fscrypt_get_policy_ex_arg -struct fscrypt_get_policy_ex_arg { - __u64 policy_size; /* input/output */ - union { - __u8 version; - struct fscrypt_policy_v1 v1; - struct fscrypt_policy_v2 v2; - } policy; /* output */ -}; -#endif - /* * Second batch of ioctls (Linux headers v5.4+), plus some renamings from FS_ to * FSCRYPT_. We don't bother defining the old names here. @@ -102,13 +73,9 @@ struct fscrypt_policy_v1 { #define FSCRYPT_POLICY_V2 2 #define FSCRYPT_KEY_IDENTIFIER_SIZE 16 -/* struct fscrypt_policy_v2 was defined earlier */ #define FSCRYPT_MAX_KEY_SIZE 64 -#define FS_IOC_GET_ENCRYPTION_POLICY_EX _IOWR('f', 22, __u8[9]) /* size + version */ -/* struct fscrypt_get_policy_ex_arg was defined earlier */ - #define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1 #define FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER 2 struct fscrypt_key_specifier { @@ -152,6 +119,40 @@ struct fscrypt_get_key_status_arg { #endif /* !FS_IOC_GET_ENCRYPTION_POLICY_EX */ +/* + * Since the log2_data_unit_size field was added later than fscrypt_policy_v2 + * itself, we may need to override the system definition to get that field. + * And also fscrypt_get_policy_ex_arg since it contains fscrypt_policy_v2. + */ +#if !defined(FS_IOC_GET_ENCRYPTION_POLICY_EX) || \ + defined(OVERRIDE_SYSTEM_FSCRYPT_POLICY_V2) +#undef fscrypt_policy_v2 +struct fscrypt_policy_v2 { + __u8 version; + __u8 contents_encryption_mode; + __u8 filenames_encryption_mode; + __u8 flags; + __u8 log2_data_unit_size; + __u8 __reserved[3]; + __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]; +}; + +#undef fscrypt_get_policy_ex_arg +struct fscrypt_get_policy_ex_arg { + __u64 policy_size; /* input/output */ + union { + __u8 version; + struct fscrypt_policy_v1 v1; + struct fscrypt_policy_v2 v2; + } policy; /* output */ +}; + +#endif + +#ifndef FS_IOC_GET_ENCRYPTION_POLICY_EX +# define FS_IOC_GET_ENCRYPTION_POLICY_EX _IOWR('f', 22, __u8[9]) /* size + version */ +#endif + /* * Since the key_id field was added later than struct fscrypt_add_key_arg * itself, we may need to override the system definition to get that field.