| Message ID | 20240816193957.42626-1-cem@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs_io: Fix fscrypt macros ordering | expand |
On Fri, Aug 16, 2024 at 09:39:38PM +0200, cem@kernel.org wrote: > #define FSCRYPT_POLICY_V2 2 > #define FSCRYPT_KEY_IDENTIFIER_SIZE 16 > /* struct fscrypt_policy_v2 was defined earlier */ The above comment needs to be removed. > +/* > + * 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 */ > +}; > + > +#define FS_IOC_GET_ENCRYPTION_POLICY_EX _IOWR('f', 22, __u8[9]) /* size + version */ > + > +#endif FS_IOC_GET_ENCRYPTION_POLICY_EX needs to be guarded by an ifdef that checks only itself, like FS_IOC_ADD_ENCRYPTION_KEY is below. > + > /* > * 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. > */ > #if !defined(FS_IOC_ADD_ENCRYPTION_KEY) || \ > defined(OVERRIDE_SYSTEM_FSCRYPT_ADD_KEY_ARG) > #undef fscrypt_add_key_arg > struct fscrypt_add_key_arg { > struct fscrypt_key_specifier key_spec; > __u32 raw_size; > __u32 key_id; > __u32 __reserved[8]; > __u8 raw[]; > }; > #endif > > #ifndef FS_IOC_ADD_ENCRYPTION_KEY > # define FS_IOC_ADD_ENCRYPTION_KEY _IOWR('f', 23, struct fscrypt_add_key_arg) > #endif Otherwise this looks good, thanks. Sorry for the trouble. It might be time to find a way to build the file without <linux/fs.h> included --- then the local definitions can just be used unconditionally. - Eric
On Fri, Aug 16, 2024 at 09:39:38PM +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> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > --- > io/encrypt.c | 64 ++++++++++++++++++++++++++-------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/io/encrypt.c b/io/encrypt.c > index 79061b07c..97abb964e 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. > @@ -106,9 +77,6 @@ struct fscrypt_policy_v1 { > > #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 +120,38 @@ 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 */ > +}; > + > +#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. > -- > 2.46.0 > >
diff --git a/io/encrypt.c b/io/encrypt.c index 79061b07c..97abb964e 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. @@ -106,9 +77,6 @@ struct fscrypt_policy_v1 { #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 +120,38 @@ 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 */ +}; + +#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.