All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy@black.fi.intel.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Justin Stitt <justinstitt@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH v2] x86/tdx: replace deprecated strncpy with strtomem_pad
Date: Wed, 7 Feb 2024 16:03:35 +0200	[thread overview]
Message-ID: <ZcONt_he_08batik@black.fi.intel.com> (raw)
In-Reply-To: <ZR0VJsgA6g0Wk4dq@gmail.com>

On Wed, Oct 04, 2023 at 09:32:54AM +0200, Ingo Molnar wrote:

...

> > Note: Ingo Molnar has some concerns about the comment being out of sync
> > [1] but I believe the comment still has a place as we can still
> > theoretically copy 64 bytes into our destination buffer without a
> > NUL-byte. The extra information about the 65th byte being NUL may serve
> > helpful to future travelers of this code. What do we think? I can drop
> > the comment in a v3 if needed.
> 
> >  	/* VMM assumes '\0' in byte 65, if the message took all 64 bytes */
> > -	strncpy(message.str, msg, 64);
> > +	strtomem_pad(message.str, msg, '\0');
> 
> My concern was that with the old code it was obvious that the size
> of message.str was 64 bytes - but I judged this based on the
> patch context alone, which seemingly lost context due to the change.
> 
> In reality it's easy to see it when reading the code, because the
> length definition is right before the code:
> 
>         union {
>                 /* Define register order according to the GHCI */
>                 struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
> 
>                 char str[64];
>                 ^^^^^^^^^^^^^
>         } message;
> 
>         /* VMM assumes '\0' in byte 65, if the message took all 64 bytes */
>         strtomem_pad(message.str, msg, '\0');

This comment and size of union seems not in agreement.
How does the previous code work if message indeed takes 64 bytes?
By luck?

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2024-02-07 20:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-03 21:54 [PATCH v2] x86/tdx: replace deprecated strncpy with strtomem_pad Justin Stitt
2023-10-03 23:44 ` Kees Cook
2023-10-03 23:46 ` Dave Hansen
2023-10-04  7:32 ` Ingo Molnar
2024-02-07 14:03   ` Andy Shevchenko [this message]
2024-02-10  7:19     ` Kees Cook
2023-10-04  7:41 ` [tip: x86/mm] x86/tdx: Replace deprecated strncpy() with strtomem_pad() tip-bot2 for Justin Stitt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZcONt_he_08batik@black.fi.intel.com \
    --to=andy@black.fi.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=justinstitt@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.