Re: [PATCH] ath10k: replace deprecated strncpy with strtomem_pad

From: Jeff Johnson
Date: Fri Oct 13 2023 - 20:58:37 EST


On 10/13/2023 1:33 PM, Justin Stitt wrote:
strncpy() is deprecated [1] and we should prefer less ambiguous
interfaces.

In this case, arvif->u.ap.ssid has its length maintained by
arvif->u.ap.ssid_len which indicates it may not need to be
NUL-terminated, although by virtue of using strtomem_pad (with NUL-byte
pad character) and having a destination size larger than the source,
ssid will, incidentally, be NUL-terminated here.

As strtomem_pad() docs say:
* @dest: Pointer of destination character array (marked as __nonstring)
* @src: Pointer to NUL-terminated string
* @pad: Padding character to fill any remaining bytes of @dest after copy
*
* This is a replacement for strncpy() uses where the destination is not
* a NUL-terminated string, but with bounds checking on the source size, and
* an explicit padding character. If padding is not required, use strtomem().

Let's also mark ath10k_vif.u.ap.ssid as __nonstring.

what criteria is used to determine whether or not to use __nonstring?
doesn't the use of u8 vs char already communicate that distinction?
just want to know what other u8 arrays might require this.
FWIW the documentation referenced by the __nonstring macro explicitly refers to "type array of char, signed char, or unsigned char"


It is unclear to me whether padding is strictly necessary. Perhaps we
should opt for just strtomem() -- padding certainly doesn't hurt,
though.

concur that padding probably isn't necessary but doesn't hurt, and will prevent confusion if looking at this member in a crashdump


Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@xxxxxxxxxxxxxxx
Signed-off-by: Justin Stitt <justinstitt@xxxxxxxxxx>

Either with or without the __nonstring...
Acked-by: Jeff Johnson <quic_jjohnson@xxxxxxxxxxx>