Re: [PATCH] xfs_io: Fix fscrypt macros ordering

[Eric Biggers <ebiggers@kernel.org>]

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