| Message ID | 20250207010022.749952-6-kees@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Annotate arguments of memtostr/strtomem with __nonstring | expand |
On 2/6/25 17:00, Kees Cook wrote: > +++ b/arch/x86/coco/tdx/tdx.c > @@ -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 <dave.hansen@linux.intel.com>
On Thu, Feb 06, 2025 at 05:12:11PM -0800, Dave Hansen wrote: > On 2/6/25 17:00, Kees Cook wrote: > > +++ b/arch/x86/coco/tdx/tdx.c > > @@ -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. Yeah, this is true all over the place. It's a string, just not a NUL-terminated string: *sob* > 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? > In any case, feel free to carry the annotation in your tree: > > Acked-by: Dave Hansen <dave.hansen@linux.intel.com> Thanks! -Kees
On Fri, Feb 7, 2025 at 4:37 AM Kees Cook <kees@kernel.org> wrote: > On Thu, Feb 06, 2025 at 05:12:11PM -0800, Dave Hansen wrote: > > On 2/6/25 17:00, Kees Cook wrote: ... > > 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 ?
On Fri, Feb 07, 2025 at 02:09:12PM +0200, Andy Shevchenko wrote: > On Fri, Feb 7, 2025 at 4:37 AM Kees Cook <kees@kernel.org> wrote: > > On Thu, Feb 06, 2025 at 05:12:11PM -0800, Dave Hansen wrote: > > > On 2/6/25 17:00, Kees Cook wrote: > > ... > > > > 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.
On Thu, Feb 06, 2025 at 06:37:27PM -0800, Kees Cook wrote: > On Thu, Feb 06, 2025 at 05:12:11PM -0800, Dave Hansen wrote: > > On 2/6/25 17:00, Kees Cook wrote: > > > +++ b/arch/x86/coco/tdx/tdx.c > > > @@ -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. > > Yeah, this is true all over the place. It's a string, just not a > NUL-terminated string: *sob* > > > 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? .bytes sounds good to me. Anyway: Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 0d9b090b4880..977ab1ffa3fe 100644 --- a/arch/x86/coco/tdx/tdx.c +++ b/arch/x86/coco/tdx/tdx.c @@ -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 */
In preparation for strtomem*() checking that its destination is a nonstring, annotate message.str accordingly. Signed-off-by: Kees Cook <kees@kernel.org> --- Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: x86@kernel.org Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: linux-coco@lists.linux.dev --- arch/x86/coco/tdx/tdx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)