| Message ID | 20231019-strncpy-drivers-net-wwan-rpmsg_wwan_ctrl-c-v2-1-ecf9b5a39430@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 75e7d0b2d22370c83c6dcb0cedbd0cca74383b5e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: wwan: replace deprecated strncpy with strscpy | expand |
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 19 Oct 2023 18:21:22 +0000 you 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. > > We expect chinfo.name to be NUL-terminated based on its use with format > strings and sprintf: > rpmsg/rpmsg_char.c > 165: dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); > 368: return sprintf(buf, "%s\n", eptdev->chinfo.name); > > [...] Here is the summary with links: - [v2] net: wwan: replace deprecated strncpy with strscpy https://git.kernel.org/netdev/net-next/c/75e7d0b2d223 You are awesome, thank you!
diff --git a/drivers/net/wwan/rpmsg_wwan_ctrl.c b/drivers/net/wwan/rpmsg_wwan_ctrl.c index 86b60aadfa11..26756ff0e44d 100644 --- a/drivers/net/wwan/rpmsg_wwan_ctrl.c +++ b/drivers/net/wwan/rpmsg_wwan_ctrl.c @@ -37,7 +37,7 @@ static int rpmsg_wwan_ctrl_start(struct wwan_port *port) .dst = RPMSG_ADDR_ANY, }; - strncpy(chinfo.name, rpwwan->rpdev->id.name, RPMSG_NAME_SIZE); + strscpy(chinfo.name, rpwwan->rpdev->id.name, sizeof(chinfo.name)); rpwwan->ept = rpmsg_create_ept(rpwwan->rpdev, rpmsg_wwan_ctrl_callback, rpwwan, chinfo); if (!rpwwan->ept)
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. We expect chinfo.name to be NUL-terminated based on its use with format strings and sprintf: rpmsg/rpmsg_char.c 165: dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); 368: return sprintf(buf, "%s\n", eptdev->chinfo.name); ... and with strcmp(): | static struct rpmsg_endpoint *qcom_glink_create_ept(struct rpmsg_device *rpdev, | rpmsg_rx_cb_t cb, | void *priv, | struct rpmsg_channel_info | chinfo) | ... | const char *name = chinfo.name; | ... | if (!strcmp(channel->name, name)) Since chinfo is initialized as such (just above the strscpy()): | struct rpmsg_channel_info chinfo = { | .src = rpwwan->rpdev->src, | .dst = RPMSG_ADDR_ANY, | }; ... we know other members are zero-initialized. This means no NUL-padding is required (as any NUL-byte assignments are redundant). Considering the above, a suitable replacement is `strscpy` due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. 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> --- Changes in v2: - clarify chinfo _is_ zero-init'd in commit msg (thanks Stephan) - use strscpy instead (thanks Kees, Stephan) - Link to v1: https://lore.kernel.org/r/20231018-strncpy-drivers-net-wwan-rpmsg_wwan_ctrl-c-v1-1-4e343270373a@google.com --- Note: found with grep Note: build-tested only I've rolled Kees' RB into this patch from v1 as it matches his previous review. --- drivers/net/wwan/rpmsg_wwan_ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 58720809f52779dc0f08e53e54b014209d13eebb change-id: 20231018-strncpy-drivers-net-wwan-rpmsg_wwan_ctrl-c-3f620aafd326 Best regards, -- Justin Stitt <justinstitt@google.com>