Annotate arguments of memtostr/strtomem with __nonstring

PATCH In review Kees Cook 5 participants 17 messages Feb 7, 2025 – Feb 13, 2025

Subsystems (10)

x86 SCSI Library code Sparse checker +6 more subsystems

lore

Share & tools

lore

Kees Cook

author maintainer

2025-02-07 01:00 UTC #1 lore raw

PATCH 1/10 scsi: mptfusion: Mark device strings as nonstring

In reply to Kees Cook

In preparation for memtostr*() checking that its source is marked as nonstring, annotate the device strings accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Sathya Prakash <[email protected]>
Cc: Sreekanth Reddy <[email protected]>
Cc: Suganath Prabu Subramani <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 drivers/message/fusion/mptsas.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/message/fusion/mptsas.c +4 -4

index d0549a4daf76..9e3a823ca4eb 100644
@@ -2834,10 +2834,10 @@ struct rep_manu_reply{
 	u8 sas_format:1;
 	u8 reserved1:7;
 	u8 reserved2[3];
-	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN];
-	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN];
-	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN];
-	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN];
+	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN] __nonstring;
+	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN] __nonstring;
+	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN] __nonstring;
+	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN] __nonstring;
 	u16 component_id;
 	u8 component_revision_id;
 	u8 reserved3;

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #0 lore raw

cover Annotate arguments of memtostr/strtomem with __nonstring

Hi,

The memtostr*() and strtomem*() helpers are designed to move between C strings (NUL-terminated) and byte arrays (that may just be zero padded and may not be NUL-terminated). The "nonstring" attribute is used to annotated these kinds of byte arrays, and we can validate the annotation on the arguments of the helpers. Add the the infrastructure to do this, and then update all the places where these annotations are currently missing.

-Kees

Kees Cook (10):
scsi: mptfusion: Mark device strings as nonstring
scsi: mpi3mr: Mark device strings as nonstring
scsi: mpt3sas: Mark device strings as nonstring
scsi: qla2xxx: Mark device strings as nonstring
string: kunit: Mark nonstring test strings as __nonstring x86/tdx: Mark message.str as nonstring
uapi: stddef.h: Introduce __kernel_nonstring
nilfs2: Mark on-disk strings as nonstring
compiler.h: Introduce __must_be_noncstr()
string.h: Validate memtostr*()/strtomem*() arguments more carefully

 arch/x86/coco/tdx/tdx.c                  |  2 +-
 drivers/message/fusion/mptsas.c          |  8 ++++----
 drivers/scsi/mpi3mr/mpi3mr_transport.c   |  8 ++++----
 drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h     |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_transport.c |  8 ++++----
 drivers/scsi/qla2xxx/qla_mr.h            |  4 ++--
 include/linux/compiler.h                 | 18 +++++++++++++++++-
 include/linux/string.h                   | 16 ++++++++++++----
 include/uapi/linux/nilfs2_ondisk.h       |  3 ++-
 include/uapi/linux/stddef.h              |  6 ++++++
 lib/string_kunit.c                       |  4 ++--
 11 files changed, 55 insertions(+), 24 deletions(-)

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #2 lore raw

PATCH 2/10 scsi: mpi3mr: Mark device strings as nonstring

In preparation for memtostr*() checking that its source is marked as nonstring, annotate the device strings accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Sathya Prakash Veerichetty <[email protected]>
Cc: Kashyap Desai <[email protected]>
Cc: Sumit Saxena <[email protected]>
Cc: Sreekanth Reddy <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/scsi/mpi3mr/mpi3mr_transport.c +4 -4

index 0ba9e6a6a13c..c8d6ced5640e 100644
@@ -105,10 +105,10 @@ struct rep_manu_reply {
 	u8 reserved0[2];
 	u8 sas_format;
 	u8 reserved2[3];
-	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN];
-	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN];
-	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN];
-	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN];
+	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN] __nonstring;
+	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN] __nonstring;
+	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN] __nonstring;
+	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN] __nonstring;
 	u16 component_id;
 	u8 component_revision_id;
 	u8 reserved3;

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #3 lore raw

PATCH 3/10 scsi: mpt3sas: Mark device strings as nonstring

In reply to Kees Cook

In preparation for memtostr*() checking that its source is marked as nonstring, annotate the device strings accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Sathya Prakash <[email protected]>
Cc: Sreekanth Reddy <[email protected]>
Cc: Suganath Prabu Subramani <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h     | 2 +-
 drivers/scsi/mpt3sas/mpt3sas_transport.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h +1 -1

index 587f7d248219..d123d3b740e1 100644
@@ -606,7 +606,7 @@ typedef struct _MPI2_CONFIG_REPLY {
 
 typedef struct _MPI2_CONFIG_PAGE_MAN_0 {
 	MPI2_CONFIG_PAGE_HEADER Header;                     /*0x00 */
-	U8                      ChipName[16];               /*0x04 */
+	U8                      ChipName[16] __nonstring;   /*0x04 */
 	U8                      ChipRevision[8];            /*0x14 */
 	U8                      BoardName[16];              /*0x1C */
 	U8                      BoardAssembly[16];          /*0x2C */

drivers/scsi/mpt3sas/mpt3sas_transport.c +4 -4

index d84413b77d84..dc74ebc6405a 100644
@@ -328,10 +328,10 @@ struct rep_manu_reply {
 	u8 reserved0[2];
 	u8 sas_format;
 	u8 reserved2[3];
-	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN];
-	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN];
-	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN];
-	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN];
+	u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN] __nonstring;
+	u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN] __nonstring;
+	u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN] __nonstring;
+	u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN] __nonstring;
 	u16 component_id;
 	u8 component_revision_id;
 	u8 reserved3;

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #4 lore raw

PATCH 4/10 scsi: qla2xxx: Mark device strings as nonstring

In reply to Kees Cook

In preparation for memtostr*() checking that its source is marked as nonstring, annotate the device strings accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Nilesh Javali <[email protected]>
Cc: [email protected]
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]

---
 drivers/scsi/qla2xxx/qla_mr.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/scsi/qla2xxx/qla_mr.h +2 -2

index 4f63aff333db..3a2bd953a976 100644
@@ -282,8 +282,8 @@ struct register_host_info {
 #define QLAFX00_TGT_NODE_LIST_SIZE (sizeof(uint32_t) * 32)
 
 struct config_info_data {
-	uint8_t		model_num[16];
-	uint8_t		model_description[80];
+	uint8_t		model_num[16] __nonstring;
+	uint8_t		model_description[80] __nonstring;
 	uint8_t		reserved0[160];
 	uint8_t		symbolic_name[64];
 	uint8_t		serial_num[32];

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #5 lore raw

PATCH 5/10 string: kunit: Mark nonstring test strings as __nonstring

In reply to Kees Cook

In preparation for strtomem*() checking that its destination is a __nonstring, annotate "nonstring" and "nonstring_small" variables accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Andy Shevchenko <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]

---
 lib/string_kunit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

lib/string_kunit.c +2 -2

index c919e3293da6..0ed7448a26d3 100644
@@ -579,8 +579,8 @@ static void string_test_strtomem(struct kunit *test)
 
 static void string_test_memtostr(struct kunit *test)
 {
-	char nonstring[7] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };
-	char nonstring_small[3] = { 'a', 'b', 'c' };
+	char nonstring[7] __nonstring = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };
+	char nonstring_small[3] __nonstring = { 'a', 'b', 'c' };
 	char dest[sizeof(nonstring) + 1];
 
 	/* Copy in a non-NUL-terminated string into exactly right-sized dest. */

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #6 lore raw

PATCH 6/10 x86/tdx: Mark message.str as nonstring

In reply to Kees Cook

In preparation for strtomem*() checking that its destination is a nonstring, annotate message.str accordingly.

Signed-off-by Kees Cook <[email protected]>

---
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: [email protected]
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]

---
 arch/x86/coco/tdx/tdx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/coco/tdx/tdx.c +1 -1

index 0d9b090b4880..977ab1ffa3fe 100644
@@ -170,7 +170,7 @@ static void __noreturn tdx_panic(const char *msg)
 		/* Define register order according to the GHCI */
 		struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
 
-		char str[64];
+		char str[64] __nonstring;
 	} message;
 
 	/* VMM assumes '\0' in byte 65, if the message took all 64 bytes */

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #12 lore raw

PATCH 7/10 uapi: stddef.h: Introduce __kernel_nonstring

In reply to Kees Cook

In order to annotate byte arrays in UAPI that are not C strings (i.e. they may not be NUL terminated), the "nonstring" attribute is needed. However, we can't expose this to userspace as it is compiler version specific.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Alexey Dobriyan <[email protected]>
Cc: Sven Eckelmann <[email protected]>
Cc: Tadeusz Struk <[email protected]>
Cc: Erick Archer <[email protected]>
Cc: Dmitry Antipov <[email protected]>

---
 include/uapi/linux/stddef.h | 6 ++++++
 1 file changed, 6 insertions(+)

include/uapi/linux/stddef.h +6 -0

index 58154117d9b0..0e7d289b7c2e 100644
@@ -63,4 +63,10 @@
 #define __counted_by_be(m)
 #endif
 
+#ifdef __KERNEL__
+#define __kernel_nonstring	__nonstring
+#else
+#define __kernel_nonstring
+#endif
+
 #endif /* _UAPI_LINUX_STDDEF_H */

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #13 lore raw

PATCH 8/10 nilfs2: Mark on-disk strings as nonstring

In reply to Kees Cook

In preparation for memtostr*() checking that its source is marked as nonstring, annotate the device strings accordingly using the new UAPI alias for the "nonstring" attribute.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Ryusuke Konishi <[email protected]>
Cc: [email protected]

---
 include/uapi/linux/nilfs2_ondisk.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/uapi/linux/nilfs2_ondisk.h +2 -1

index c23f91ae5fe8..3196cc44a002 100644
@@ -188,7 +188,8 @@ struct nilfs_super_block {
 	__le16	s_segment_usage_size;	/* Size of a segment usage */
 
 /*98*/	__u8	s_uuid[16];		/* 128-bit uuid for volume */
-/*A8*/	char	s_volume_name[80];	/* volume name */
+/*A8*/	char	s_volume_name[80]	/* volume name */
+			__kernel_nonstring;
 
 /*F8*/	__le32  s_c_interval;           /* Commit interval of segment */
 	__le32  s_c_block_max;          /*

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #14 lore raw

PATCH 9/10 compiler.h: Introduce __must_be_noncstr()

In reply to Kees Cook

In preparation for adding more type checking to the memtostr/strtomem*() helpers, introduce the ability to check for the "nonstring" attribute. This is the reverse of what was added to strscpy*() in commit 559048d156ff ("string: Check for "nonstring" attribute on strscpy() arguments").

Signed-off-by Kees Cook <[email protected]>

---
 include/linux/compiler.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

include/linux/compiler.h +17 -1

index 1c0688319435..c89070a2f964 100644
@@ -229,9 +229,25 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 #define __must_be_byte_array(a)	__BUILD_BUG_ON_ZERO_MSG(!__is_byte_array(a), \
 							"must be byte array")
 
+/*
+ * If the "nonstring" attribute isn't available, we have to return true
+ * so the __must_*() checks pass when "nonstring" isn't supported.
+ */
+#if __has_attribute(__nonstring__)
+#define __is_cstr(a)		(!__annotated(a, nonstring))
+#define __is_noncstr(a)		(__annotated(a, nonstring))
+#else
+#define __is_cstr(a)		(true)
+#define __is_noncstr(a)		(true)
+#endif
+
 /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */
 #define __must_be_cstr(p) \
-	__BUILD_BUG_ON_ZERO_MSG(__annotated(p, nonstring), "must be cstr (NUL-terminated)")
+	__BUILD_BUG_ON_ZERO_MSG(!__is_cstr(p), \
+				"must be C-string (NUL-terminated)")
+#define __must_be_noncstr(p) \
+	__BUILD_BUG_ON_ZERO_MSG(!__is_noncstr(p), \
+				"must be non-C-string (not NUL-terminated)")
 
 #endif /* __KERNEL__ */

Signature · 1 line

2.34.1

Kees Cook

author maintainer

2025-02-07 01:00 UTC #15 lore raw

PATCH 10/10 string.h: Validate memtostr*()/strtomem*() arguments more carefully

In reply to Kees Cook

Since these functions handle moving between C strings and non-C strings, they should check for the appropriate presence/lack of the nonstring attribute on arguments.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Andy Shevchenko <[email protected]>
Cc: [email protected]

---
 include/linux/string.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

include/linux/string.h +12 -4

index fc5ae145bd78..26491a2f8010 100644
@@ -412,8 +412,10 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
  */
 #define strtomem_pad(dest, src, pad)	do {				\
 	const size_t _dest_len = __must_be_byte_array(dest) +		\
+				 __must_be_noncstr(dest) +		\
 				 ARRAY_SIZE(dest);			\
-	const size_t _src_len = __builtin_object_size(src, 1);		\
+	const size_t _src_len = __must_be_cstr(src) +			\
+				__builtin_object_size(src, 1);		\
 									\
 	BUILD_BUG_ON(!__builtin_constant_p(_dest_len) ||		\
 		     _dest_len == (size_t)-1);				\

@@ -436,8 +438,10 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
  */
 #define strtomem(dest, src)	do {					\
 	const size_t _dest_len = __must_be_byte_array(dest) +		\
+				 __must_be_noncstr(dest) +		\
 				 ARRAY_SIZE(dest);			\
-	const size_t _src_len = __builtin_object_size(src, 1);		\
+	const size_t _src_len = __must_be_cstr(src) +			\
+				__builtin_object_size(src, 1);		\
 									\
 	BUILD_BUG_ON(!__builtin_constant_p(_dest_len) ||		\
 		     _dest_len == (size_t)-1);				\

@@ -456,8 +460,10 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
  */
 #define memtostr(dest, src)	do {					\
 	const size_t _dest_len = __must_be_byte_array(dest) +		\
+				 __must_be_cstr(dest) +			\
 				 ARRAY_SIZE(dest);			\
-	const size_t _src_len = __builtin_object_size(src, 1);		\
+	const size_t _src_len = __must_be_noncstr(src) +		\
+				__builtin_object_size(src, 1);		\
 	const size_t _src_chars = strnlen(src, _src_len);		\
 	const size_t _copy_len = min(_dest_len - 1, _src_chars);	\
 									\

@@ -482,8 +488,10 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
  */
 #define memtostr_pad(dest, src)		do {				\
 	const size_t _dest_len = __must_be_byte_array(dest) +		\
+				 __must_be_cstr(dest) +			\
 				 ARRAY_SIZE(dest);			\
-	const size_t _src_len = __builtin_object_size(src, 1);		\
+	const size_t _src_len = __must_be_noncstr(src) +		\
+				__builtin_object_size(src, 1);		\
 	const size_t _src_chars = strnlen(src, _src_len);		\
 	const size_t _copy_len = min(_dest_len - 1, _src_chars);	\
 									\

Signature · 1 line

2.34.1

Dave Hansen

Acked-by on v1 6/10

2025-02-07 01:12 UTC #7 lore raw

Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

In reply to Kees Cook

arch/x86/coco/tdx/tdx.c static void __noreturn tdx_panic(const char *msg) · line 174

Quoted 8 lines · Kees Cook

+++ b/arch/x86/coco/tdx/tdx.c

arch/x86/coco/tdx/tdx.c +1 -1

@@ -170,7 +170,7 @@ static void __noreturn tdx_panic(const char *msg)
 		/* Define register order according to the GHCI */
 		struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
 
-		char str[64];
+		char str[64] __nonstring;
 	} message;

So, the patch itself makes sense. But it does end up looking kinda funky. We call it a "str"ing and then annotate it as not a string.

It doesn't have to be done in this patch, but it does seem like we should probably not be using 'char' and also shouldn't call it anything close to "string". Maybe:

	u8 message[64] __nonstring;

In any case, feel free to carry the annotation in your tree:

Acked-by Dave Hansen <[email protected]>

Kees Cook

author maintainer

2025-02-07 02:37 UTC #8 lore raw

Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

Quoted 6 lines · Dave Hansen

Quoted 5 lines · Kees Cook

+++ b/arch/x86/coco/tdx/tdx.c

arch/x86/coco/tdx/tdx.c +0 -0

@@ -170,7 +170,7 @@ static void __noreturn tdx_panic(const char *msg)
 		/* Define register order according to the GHCI */
 		struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };

Quoted 6 lines

arch/x86/coco/tdx/tdx.c static void __noreturn tdx_panic(const char *msg) · line 174
Quoted 3 lines
arch/x86/coco/tdx/tdx.c +1 -1
-		char str[64];
+		char str[64] __nonstring;
 	} message;
So, the patch itself makes sense. But it does end up looking kinda funky. We call it a "str"ing and then annotate it as not a string.

Yeah, this is true all over the place. It's a string, just not a NUL-terminated string: *sob*

Quoted 6 lines

It doesn't have to be done in this patch, but it does seem like we should probably not be using 'char' and also shouldn't call it anything close to "string". Maybe:
	 	u8 message[64] __nonstring;
	} message;

message.message ;)

message.chars?
message.bytes?

Quoted 3 lines

In any case, feel free to carry the annotation in your tree:
Acked-by Dave Hansen <[email protected]>

Thanks!

-Kees

Signature · 1 line

Kees Cook

Andy Shevchenko

2025-02-07 12:09 UTC #9 lore raw

Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

Quoted 2 lines · Kees Cook

Quoted 1 line · Dave Hansen
On 2/6/25 17:00, Kees Cook wrote:

...

Quoted 5 lines

Quoted 2 lines
So, the patch itself makes sense. But it does end up looking kinda funky. We call it a "str"ing and then annotate it as not a string.
Yeah, this is true all over the place. It's a string, just not a NUL-terminated string: *sob*

Maybe call it respectively, e.g., __nontermstr ?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Kees Cook

author maintainer

2025-02-08 21:16 UTC #10 lore raw

Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

Quoted 6 lines · Andy Shevchenko

Quoted 2 lines · Kees Cook
Quoted 1 line · Dave Hansen
On 2/6/25 17:00, Kees Cook wrote:
...

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
So, the patch itself makes sense. But it does end up looking kinda funky. We call it a "str"ing and then annotate it as not a string.
Yeah, this is true all over the place. It's a string, just not a NUL-terminated string: *sob*
Maybe call it respectively, e.g., __nontermstr ?

I don't want to change its name from the GCC attribute. I think that's just asking more more confusion.

Signature · 1 line

Kees Cook

Kirill A. Shutemov

Acked-by

2025-02-10 11:54 UTC #11 lore raw

Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

In reply to Kees Cook

Quoted 21 lines · Kees Cook

Quoted 6 lines · Dave Hansen
Quoted 5 lines · Kees Cook
+++ b/arch/x86/coco/tdx/tdx.c
arch/x86/coco/tdx/tdx.c +0 -0
@@ -170,7 +170,7 @@ static void __noreturn tdx_panic(const char *msg)
 		/* Define register order according to the GHCI */
 		struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
 
Quoted 6 lines
arch/x86/coco/tdx/tdx.c static void __noreturn tdx_panic(const char *msg) · line 174
Quoted 3 lines
arch/x86/coco/tdx/tdx.c +1 -1
-		char str[64];
+		char str[64] __nonstring;
 	} message;
So, the patch itself makes sense. But it does end up looking kinda funky. We call it a "str"ing and then annotate it as not a string.
Yeah, this is true all over the place. It's a string, just not a NUL-terminated string: *sob*
Quoted 4 lines
It doesn't have to be done in this patch, but it does seem like we should probably not be using 'char' and also shouldn't call it anything close to "string". Maybe:

Quoted 7 lines

Quoted 2 lines
	 	u8 message[64] __nonstring;
	} message;
message.message ;)
message.chars?
message.bytes?

.bytes sounds good to me.

Anyway:

Acked-by Kirill A. Shutemov <[email protected]>

Signature · 1 line

  Kiryl Shutsemau / Kirill A. Shutemov

Martin K. Petersen

Reviewed-by on the v1 series

2025-02-13 03:04 UTC #16 lore raw

In reply to Kees Cook

Kees,

Quoted 7 lines

The memtostr*() and strtomem*() helpers are designed to move between C strings (NUL-terminated) and byte arrays (that may just be zero padded and may not be NUL-terminated). The "nonstring" attribute is used to annotated these kinds of byte arrays, and we can validate the annotation on the arguments of the helpers. Add the the infrastructure to do this, and then update all the places where these annotations are currently missing.

Reviewed-by Martin K. Petersen <[email protected]> # SCSI

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering