On Mon, Sep 11, 2023 at 03:01:16PM -0700, Justin Stitt wrote: > On Mon, Sep 11, 2023 at 11:51 AM Dave Hansen <dave.han...@intel.com> wrote: > > > > On 9/11/23 11:27, Justin Stitt wrote: > > > `strncpy` is deprecated and we should prefer more robust string apis. > > > > I dunno. It actually seems like a pretty good fit here. > > > > > In this case, `message.str` is not expected to be NUL-terminated as it > > > is simply a buffer of characters residing in a union which allows for > > > named fields representing 8 bytes each. There is only one caller of > > > `tdx_panic()` and they use a 59-length string for `msg`: > > > | const char *msg = "TD misconfiguration: SEPT_VE_DISABLE attribute > > > must be set."; > > > > I'm not really following this logic. > > > > We need to do the following: > > > > 1. Make sure not to over write past the end of 'message' > > 2. Make sure not to over read past the end of 'msg' > > 3. Make sure not to leak stack data into the hypercall registers > > in the case of short strings. > > > > strncpy() does #1, #2 and #3 just fine. > > Right, to be clear, I do not suspect a bug in the current > implementation. Rather, let's move towards a less ambiguous interface > for maintainability's sake > > > > > The resulting string does *NOT* need to be NULL-terminated. See the > > comment: > > > > /* VMM assumes '\0' in byte 65, if the message took all 64 bytes */ > > > > Are there cases where strncpy() doesn't NULL-terminate the string other > > than when the buffer is full? > > > > I actually didn't realize that strncpy() pads its output up to the full > > size. I wonder if Kirill used it intentionally or whether he got lucky > > here. :) > > Big reason to use strtomem_pad as it is more obvious about what it does. > > I'd love more thoughts/testing here.
This looks like exactly the right conversion: strtomem_pad() will do 1, 2, and 3 (and does it unambiguously and without allowing for a possible-wrong "size" parameter for the destination buffer). Reviewed-by: Kees Cook <keesc...@chromium.org> -- Kees Cook
