From: Kalle Valo <kvalo@kernel.org>
To: Justin Stitt <justinstitt@google.com>
Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org,
Justin Stitt <justinstitt@google.com>
Subject: Re: [PATCH v2] wifi: ipw2x00: replace deprecated strncpy with strscpy_pad
Date: Mon, 23 Oct 2023 17:26:49 +0000 (UTC) [thread overview]
Message-ID: <169808200764.695306.10833187895555987178.kvalo@kernel.org> (raw)
In-Reply-To: <20231017-strncpy-drivers-net-wireless-intel-ipw2x00-ipw2200-c-v2-1-465e10dc817c@google.com>
Justin Stitt <justinstitt@google.com> wrote:
> strncpy() is deprecated for use on NUL-terminated destination strings
> [1] and as such we should prefer more robust and less ambiguous string
> interfaces.
>
> `extra` is intended to be NUL-terminated which is evident by the manual
> assignment of a NUL-byte as well as its immediate usage with strlen().
>
> Moreover, many of these getters and setters are NUL-padding buffers with
> memset():
> 2439 | memset(&tx_power, 0, sizeof(tx_power));
> 9998 | memset(sys_config, 0, sizeof(struct ipw_sys_config));
> 10084 | memset(tfd, 0, sizeof(*tfd));
> 10261 | memset(&dummystats, 0, sizeof(dummystats));
> ... let's maintain this behavior and NUL-pad our destination buffer.
>
> Considering the above, a suitable replacement is `strscpy_pad` due to
> the fact that it guarantees both NUL-termination and NUL-padding on the
> destination buffer.
>
> To be clear, there is no bug in the current implementation as
> MAX_WX_STRING is much larger than the size of the string literals being
> copied from. Also, strncpy() does NUL-pad the destination buffer and
> using strscpy_pad() simply matches that behavior. All in all, there
> should be no functional change but we are one step closer to eliminating
> usage of strncpy().
>
> Do note that we cannot use the more idiomatic strscpy invocation of
> (dest, src, sizeof(dest)) as the destination buffer cannot have its size
> determined at compile time. So, let's stick with (dest, src, LEN).
>
> 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@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
Patch applied to wireless-next.git, thanks.
8890b9bca38f wifi: ipw2x00: replace deprecated strncpy with strscpy_pad
--
https://patchwork.kernel.org/project/linux-wireless/patch/20231017-strncpy-drivers-net-wireless-intel-ipw2x00-ipw2200-c-v2-1-465e10dc817c@google.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
prev parent reply other threads:[~2023-10-23 17:26 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-17 21:48 [PATCH v2] wifi: ipw2x00: replace deprecated strncpy with strscpy_pad Justin Stitt
2023-10-19 1:19 ` Kees Cook
2023-10-23 17:26 ` Kalle Valo [this message]
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=169808200764.695306.10833187895555987178.kvalo@kernel.org \
--to=kvalo@kernel.org \
--cc=justinstitt@google.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=stas.yakovlev@gmail.com \
/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.