diff mbox series

[f2fs-dev,v4] f2fs-tools: support to show fscrypt_context_v2 in print_xattr_entry

Message ID 20230715060936.2343780-1-guoweichao@oppo.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev,v4] f2fs-tools: support to show fscrypt_context_v2 in print_xattr_entry | expand

Commit Message

Weichao Guo July 15, 2023, 6:09 a.m. UTC 
As the fscrypt context has two versions now, this patch adds the
support of fscrypt_context_v2 for print_xattr_entry.

Signed-off-by: Weichao Guo <guoweichao@oppo.com>
Signed-off-by: Sheng Yong <shengyong@oppo.com>
---
 fsck/mount.c | 48 ++++++++++++++++++++++++++++++----------------
 fsck/xattr.h | 54 +++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 77 insertions(+), 25 deletions(-)

Comments

Chao Yu July 17, 2023, 1:20 a.m. UTC | #1 
On 2023/7/15 14:09, Weichao Guo wrote:
> As the fscrypt context has two versions now, this patch adds the
> support of fscrypt_context_v2 for print_xattr_entry.
> 
> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
> Signed-off-by: Sheng Yong <shengyong@oppo.com>
> ---
>   fsck/mount.c | 48 ++++++++++++++++++++++++++++++----------------
>   fsck/xattr.h | 54 +++++++++++++++++++++++++++++++++++++++++++---------
>   2 files changed, 77 insertions(+), 25 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index df0314d..97d56f4 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -194,7 +194,7 @@ static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
>   {
>   	const u8 *value = (const u8 *)&ent->e_name[ent->e_name_len];
>   	const int size = le16_to_cpu(ent->e_value_size);
> -	const struct fscrypt_context *ctx;
> +	const union fscrypt_context *ctx;
>   	int i;
>   
>   	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
> @@ -211,22 +211,38 @@ static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
>   		return;
>   #endif
>   	case F2FS_XATTR_INDEX_ENCRYPTION:

#define F2FS_XATTR_NAME_ENCRYPTION_CONTEXT	"c"

Should we check ent->e_name and ent->e_name_len w/ F2FS_XATTR_NAME_ENCRYPTION_CONTEXT?

Thanks,

> -		ctx = (const struct fscrypt_context *)value;
> -		if (size != sizeof(*ctx) ||
> -		    ctx->format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
> +		ctx = (const union fscrypt_context *)value;
> +		if (size == 0 || size != fscrypt_context_size(ctx))
>   			break;
> -		MSG(0, "format: %d\n", ctx->format);
> -		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
> -		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
> -		MSG(0, "flags: 0x%x\n", ctx->flags);
> -		MSG(0, "master_key_descriptor: ");
> -		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
> -			MSG(0, "%02X", ctx->master_key_descriptor[i]);
> -		MSG(0, "\nnonce: ");
> -		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
> -			MSG(0, "%02X", ctx->nonce[i]);
> -		MSG(0, "\n");
> -		return;
> +		switch (ctx->version) {
> +		case FSCRYPT_CONTEXT_V1:
> +			MSG(0, "format: %d\n", ctx->version);
> +			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v1.contents_encryption_mode);
> +			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v1.filenames_encryption_mode);
> +			MSG(0, "flags: 0x%x\n", ctx->v1.flags);
> +			MSG(0, "master_key_descriptor: ");
> +			for (i = 0; i < FSCRYPT_KEY_DESCRIPTOR_SIZE; i++)
> +				MSG(0, "%02X", ctx->v1.master_key_descriptor[i]);
> +			MSG(0, "\nnonce: ");
> +			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
> +				MSG(0, "%02X", ctx->v1.nonce[i]);
> +			MSG(0, "\n");
> +			return;
> +		case FSCRYPT_CONTEXT_V2:
> +			MSG(0, "format: %d\n", ctx->version);
> +			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v2.contents_encryption_mode);
> +			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v2.filenames_encryption_mode);
> +			MSG(0, "flags: 0x%x\n", ctx->v2.flags);
> +			MSG(0, "master_key_identifier: ");
> +			for (i = 0; i < FSCRYPT_KEY_IDENTIFIER_SIZE; i++)
> +				MSG(0, "%02X", ctx->v2.master_key_identifier[i]);
> +			MSG(0, "\nnonce: ");
> +			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
> +				MSG(0, "%02X", ctx->v2.nonce[i]);
> +			MSG(0, "\n");
> +			return;
> +		}
> +		break;
>   	}
>   	for (i = 0; i < size; i++)
>   		MSG(0, "%02X", value[i]);
> diff --git a/fsck/xattr.h b/fsck/xattr.h
> index 22ea35c..b155cc8 100644
> --- a/fsck/xattr.h
> +++ b/fsck/xattr.h
> @@ -34,22 +34,58 @@ struct f2fs_xattr_entry {
>   	char e_name[0];		/* attribute name */
>   };
>   
> -#define FS_ENCRYPTION_CONTEXT_FORMAT_V1 1
> -#ifndef FS_KEY_DESCRIPTOR_SIZE
> -#define FS_KEY_DESCRIPTOR_SIZE 8
> +#define FSCRYPT_CONTEXT_V1 1
> +#define FSCRYPT_CONTEXT_V2 2
> +#ifndef FSCRYPT_KEY_DESCRIPTOR_SIZE
> +#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
>   #endif
> -#define FS_KEY_DERIVATION_NONCE_SIZE 16
> +#ifndef FSCRYPT_KEY_IDENTIFIER_SIZE
> +#define FSCRYPT_KEY_IDENTIFIER_SIZE	16
> +#endif
> +#define FSCRYPT_FILE_NONCE_SIZE 16
> +
> +struct fscrypt_context_v1 {
> +	u8 version; /* FSCRYPT_CONTEXT_V1 */
> +	u8 contents_encryption_mode;
> +	u8 filenames_encryption_mode;
> +	u8 flags;
> +	u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> +	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
> +};
>   
> -struct fscrypt_context {
> -	u8 format;
> +struct fscrypt_context_v2 {
> +	u8 version; /* FSCRYPT_CONTEXT_V2 */
>   	u8 contents_encryption_mode;
>   	u8 filenames_encryption_mode;
>   	u8 flags;
> -	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> -	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
> +	u8 __reserved[4];
> +	u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
> +	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
>   };
>   
> -static_assert(sizeof(struct fscrypt_context) == 28, "");
> +union fscrypt_context {
> +	u8 version;
> +	struct fscrypt_context_v1 v1;
> +	struct fscrypt_context_v2 v2;
> +};
> +
> +static_assert(sizeof(struct fscrypt_context_v1) == 28, "");
> +static_assert(sizeof(struct fscrypt_context_v2) == 40, "");
> +
> +/*
> +* Return the size expected for the given fscrypt_context based on its version
> +* number, or 0 if the context version is unrecognized.
> +*/
> +static inline int fscrypt_context_size(const union fscrypt_context *ctx)
> +{
> +	switch (ctx->version) {
> +	case FSCRYPT_CONTEXT_V1:
> +		return sizeof(ctx->v1);
> +	case FSCRYPT_CONTEXT_V2:
> +		return sizeof(ctx->v2);
> +	}
> +	return 0;
> +}
>   
>   #define F2FS_ACL_VERSION	0x0001
>
Chao Yu July 17, 2023, 1:40 a.m. UTC | #2 
On 2023/7/15 14:09, Weichao Guo wrote:
> As the fscrypt context has two versions now, this patch adds the
> support of fscrypt_context_v2 for print_xattr_entry.
> 
> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
> Signed-off-by: Sheng Yong <shengyong@oppo.com>
> ---
>   fsck/mount.c | 48 ++++++++++++++++++++++++++++++----------------
>   fsck/xattr.h | 54 +++++++++++++++++++++++++++++++++++++++++++---------
>   2 files changed, 77 insertions(+), 25 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index df0314d..97d56f4 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -194,7 +194,7 @@ static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
>   {
>   	const u8 *value = (const u8 *)&ent->e_name[ent->e_name_len];
>   	const int size = le16_to_cpu(ent->e_value_size);
> -	const struct fscrypt_context *ctx;
> +	const union fscrypt_context *ctx;
>   	int i;
>   
>   	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
> @@ -211,22 +211,38 @@ static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
>   		return;
>   #endif
>   	case F2FS_XATTR_INDEX_ENCRYPTION:
> -		ctx = (const struct fscrypt_context *)value;
> -		if (size != sizeof(*ctx) ||
> -		    ctx->format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
> +		ctx = (const union fscrypt_context *)value;
> +		if (size == 0 || size != fscrypt_context_size(ctx))

If I understand Eric's commnets correctly, if size == 0, we'd better just return in
order to avoid below codes which may access out-of-boundary on xattr entry.

for (i = 0; i < size; i++)
	MSG(0, "%02X", value[i]);

Thanks,

>   			break;
> -		MSG(0, "format: %d\n", ctx->format);
> -		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
> -		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
> -		MSG(0, "flags: 0x%x\n", ctx->flags);
> -		MSG(0, "master_key_descriptor: ");
> -		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
> -			MSG(0, "%02X", ctx->master_key_descriptor[i]);
> -		MSG(0, "\nnonce: ");
> -		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
> -			MSG(0, "%02X", ctx->nonce[i]);
> -		MSG(0, "\n");
> -		return;
> +		switch (ctx->version) {
> +		case FSCRYPT_CONTEXT_V1:
> +			MSG(0, "format: %d\n", ctx->version);
> +			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v1.contents_encryption_mode);
> +			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v1.filenames_encryption_mode);
> +			MSG(0, "flags: 0x%x\n", ctx->v1.flags);
> +			MSG(0, "master_key_descriptor: ");
> +			for (i = 0; i < FSCRYPT_KEY_DESCRIPTOR_SIZE; i++)
> +				MSG(0, "%02X", ctx->v1.master_key_descriptor[i]);
> +			MSG(0, "\nnonce: ");
> +			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
> +				MSG(0, "%02X", ctx->v1.nonce[i]);
> +			MSG(0, "\n");
> +			return;
> +		case FSCRYPT_CONTEXT_V2:
> +			MSG(0, "format: %d\n", ctx->version);
> +			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v2.contents_encryption_mode);
> +			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v2.filenames_encryption_mode);
> +			MSG(0, "flags: 0x%x\n", ctx->v2.flags);
> +			MSG(0, "master_key_identifier: ");
> +			for (i = 0; i < FSCRYPT_KEY_IDENTIFIER_SIZE; i++)
> +				MSG(0, "%02X", ctx->v2.master_key_identifier[i]);
> +			MSG(0, "\nnonce: ");
> +			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
> +				MSG(0, "%02X", ctx->v2.nonce[i]);
> +			MSG(0, "\n");
> +			return;
> +		}
> +		break;
>   	}
>   	for (i = 0; i < size; i++)
>   		MSG(0, "%02X", value[i]);
> diff --git a/fsck/xattr.h b/fsck/xattr.h
> index 22ea35c..b155cc8 100644
> --- a/fsck/xattr.h
> +++ b/fsck/xattr.h
> @@ -34,22 +34,58 @@ struct f2fs_xattr_entry {
>   	char e_name[0];		/* attribute name */
>   };
>   
> -#define FS_ENCRYPTION_CONTEXT_FORMAT_V1 1
> -#ifndef FS_KEY_DESCRIPTOR_SIZE
> -#define FS_KEY_DESCRIPTOR_SIZE 8
> +#define FSCRYPT_CONTEXT_V1 1
> +#define FSCRYPT_CONTEXT_V2 2
> +#ifndef FSCRYPT_KEY_DESCRIPTOR_SIZE
> +#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
>   #endif
> -#define FS_KEY_DERIVATION_NONCE_SIZE 16
> +#ifndef FSCRYPT_KEY_IDENTIFIER_SIZE
> +#define FSCRYPT_KEY_IDENTIFIER_SIZE	16
> +#endif
> +#define FSCRYPT_FILE_NONCE_SIZE 16
> +
> +struct fscrypt_context_v1 {
> +	u8 version; /* FSCRYPT_CONTEXT_V1 */
> +	u8 contents_encryption_mode;
> +	u8 filenames_encryption_mode;
> +	u8 flags;
> +	u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> +	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
> +};
>   
> -struct fscrypt_context {
> -	u8 format;
> +struct fscrypt_context_v2 {
> +	u8 version; /* FSCRYPT_CONTEXT_V2 */
>   	u8 contents_encryption_mode;
>   	u8 filenames_encryption_mode;
>   	u8 flags;
> -	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> -	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
> +	u8 __reserved[4];
> +	u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
> +	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
>   };
>   
> -static_assert(sizeof(struct fscrypt_context) == 28, "");
> +union fscrypt_context {
> +	u8 version;
> +	struct fscrypt_context_v1 v1;
> +	struct fscrypt_context_v2 v2;
> +};
> +
> +static_assert(sizeof(struct fscrypt_context_v1) == 28, "");
> +static_assert(sizeof(struct fscrypt_context_v2) == 40, "");
> +
> +/*
> +* Return the size expected for the given fscrypt_context based on its version
> +* number, or 0 if the context version is unrecognized.
> +*/
> +static inline int fscrypt_context_size(const union fscrypt_context *ctx)
> +{
> +	switch (ctx->version) {
> +	case FSCRYPT_CONTEXT_V1:
> +		return sizeof(ctx->v1);
> +	case FSCRYPT_CONTEXT_V2:
> +		return sizeof(ctx->v2);
> +	}
> +	return 0;
> +}
>   
>   #define F2FS_ACL_VERSION	0x0001
>
Chao Yu July 17, 2023, 2:54 p.m. UTC | #3 
On 2023/7/17 9:40, Chao Yu wrote:
> If I understand Eric's commnets correctly, if size == 0, we'd better just return in
> order to avoid below codes which may access out-of-boundary on xattr entry.

Oh, wrong comment, please ignore it.

Thanks,
diff mbox series

Patch

diff --git a/fsck/mount.c b/fsck/mount.c
index df0314d..97d56f4 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -194,7 +194,7 @@  static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
 {
 	const u8 *value = (const u8 *)&ent->e_name[ent->e_name_len];
 	const int size = le16_to_cpu(ent->e_value_size);
-	const struct fscrypt_context *ctx;
+	const union fscrypt_context *ctx;
 	int i;
 
 	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
@@ -211,22 +211,38 @@  static void print_xattr_entry(const struct f2fs_xattr_entry *ent)
 		return;
 #endif
 	case F2FS_XATTR_INDEX_ENCRYPTION:
-		ctx = (const struct fscrypt_context *)value;
-		if (size != sizeof(*ctx) ||
-		    ctx->format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+		ctx = (const union fscrypt_context *)value;
+		if (size == 0 || size != fscrypt_context_size(ctx))
 			break;
-		MSG(0, "format: %d\n", ctx->format);
-		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
-		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
-		MSG(0, "flags: 0x%x\n", ctx->flags);
-		MSG(0, "master_key_descriptor: ");
-		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
-			MSG(0, "%02X", ctx->master_key_descriptor[i]);
-		MSG(0, "\nnonce: ");
-		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
-			MSG(0, "%02X", ctx->nonce[i]);
-		MSG(0, "\n");
-		return;
+		switch (ctx->version) {
+		case FSCRYPT_CONTEXT_V1:
+			MSG(0, "format: %d\n", ctx->version);
+			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v1.contents_encryption_mode);
+			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v1.filenames_encryption_mode);
+			MSG(0, "flags: 0x%x\n", ctx->v1.flags);
+			MSG(0, "master_key_descriptor: ");
+			for (i = 0; i < FSCRYPT_KEY_DESCRIPTOR_SIZE; i++)
+				MSG(0, "%02X", ctx->v1.master_key_descriptor[i]);
+			MSG(0, "\nnonce: ");
+			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
+				MSG(0, "%02X", ctx->v1.nonce[i]);
+			MSG(0, "\n");
+			return;
+		case FSCRYPT_CONTEXT_V2:
+			MSG(0, "format: %d\n", ctx->version);
+			MSG(0, "contents_encryption_mode: 0x%x\n", ctx->v2.contents_encryption_mode);
+			MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->v2.filenames_encryption_mode);
+			MSG(0, "flags: 0x%x\n", ctx->v2.flags);
+			MSG(0, "master_key_identifier: ");
+			for (i = 0; i < FSCRYPT_KEY_IDENTIFIER_SIZE; i++)
+				MSG(0, "%02X", ctx->v2.master_key_identifier[i]);
+			MSG(0, "\nnonce: ");
+			for (i = 0; i < FSCRYPT_FILE_NONCE_SIZE; i++)
+				MSG(0, "%02X", ctx->v2.nonce[i]);
+			MSG(0, "\n");
+			return;
+		}
+		break;
 	}
 	for (i = 0; i < size; i++)
 		MSG(0, "%02X", value[i]);
diff --git a/fsck/xattr.h b/fsck/xattr.h
index 22ea35c..b155cc8 100644
--- a/fsck/xattr.h
+++ b/fsck/xattr.h
@@ -34,22 +34,58 @@  struct f2fs_xattr_entry {
 	char e_name[0];		/* attribute name */
 };
 
-#define FS_ENCRYPTION_CONTEXT_FORMAT_V1 1
-#ifndef FS_KEY_DESCRIPTOR_SIZE
-#define FS_KEY_DESCRIPTOR_SIZE 8
+#define FSCRYPT_CONTEXT_V1 1
+#define FSCRYPT_CONTEXT_V2 2
+#ifndef FSCRYPT_KEY_DESCRIPTOR_SIZE
+#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
 #endif
-#define FS_KEY_DERIVATION_NONCE_SIZE 16
+#ifndef FSCRYPT_KEY_IDENTIFIER_SIZE
+#define FSCRYPT_KEY_IDENTIFIER_SIZE	16
+#endif
+#define FSCRYPT_FILE_NONCE_SIZE 16
+
+struct fscrypt_context_v1 {
+	u8 version; /* FSCRYPT_CONTEXT_V1 */
+	u8 contents_encryption_mode;
+	u8 filenames_encryption_mode;
+	u8 flags;
+	u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
+};
 
-struct fscrypt_context {
-	u8 format;
+struct fscrypt_context_v2 {
+	u8 version; /* FSCRYPT_CONTEXT_V2 */
 	u8 contents_encryption_mode;
 	u8 filenames_encryption_mode;
 	u8 flags;
-	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
-	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+	u8 __reserved[4];
+	u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
 };
 
-static_assert(sizeof(struct fscrypt_context) == 28, "");
+union fscrypt_context {
+	u8 version;
+	struct fscrypt_context_v1 v1;
+	struct fscrypt_context_v2 v2;
+};
+
+static_assert(sizeof(struct fscrypt_context_v1) == 28, "");
+static_assert(sizeof(struct fscrypt_context_v2) == 40, "");
+
+/*
+* Return the size expected for the given fscrypt_context based on its version
+* number, or 0 if the context version is unrecognized.
+*/
+static inline int fscrypt_context_size(const union fscrypt_context *ctx)
+{
+	switch (ctx->version) {
+	case FSCRYPT_CONTEXT_V1:
+		return sizeof(ctx->v1);
+	case FSCRYPT_CONTEXT_V2:
+		return sizeof(ctx->v2);
+	}
+	return 0;
+}
 
 #define F2FS_ACL_VERSION	0x0001