* [PATCH] xfs_io: Fix fscrypt macros ordering
@ 2024-08-16 19:39 cem
2024-08-16 20:05 ` Eric Biggers
2024-08-16 20:20 ` Bill O'Donnell
0 siblings, 2 replies; 3+ messages in thread
From: cem @ 2024-08-16 19:39 UTC (permalink / raw)
To: linux-xfs; +Cc: ebiggers, wbx
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>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] xfs_io: Fix fscrypt macros ordering
2024-08-16 19:39 [PATCH] xfs_io: Fix fscrypt macros ordering cem
@ 2024-08-16 20:05 ` Eric Biggers
2024-08-16 20:20 ` Bill O'Donnell
1 sibling, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2024-08-16 20:05 UTC (permalink / raw)
To: cem; +Cc: linux-xfs, wbx
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xfs_io: Fix fscrypt macros ordering
2024-08-16 19:39 [PATCH] xfs_io: Fix fscrypt macros ordering cem
2024-08-16 20:05 ` Eric Biggers
@ 2024-08-16 20:20 ` Bill O'Donnell
1 sibling, 0 replies; 3+ messages in thread
From: Bill O'Donnell @ 2024-08-16 20:20 UTC (permalink / raw)
To: cem; +Cc: linux-xfs, ebiggers, wbx
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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-16 20:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-16 19:39 [PATCH] xfs_io: Fix fscrypt macros ordering cem
2024-08-16 20:05 ` Eric Biggers
2024-08-16 20:20 ` Bill O'Donnell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox