-
Justin Stitt authored
Let's move away from using strncpy and instead favor a less ambiguous and more robust interface. For ifp->ndev->name, we expect ifp->ndev->name to be NUL-terminated based on its use in format strings within core.c: 67 | char *brcmf_ifname(struct brcmf_if *ifp) 68 | { 69 | if (!ifp) 70 | return "<if_null>"; 71 | 72 | if (ifp->ndev) 73 | return ifp->ndev->name; 74 | 75 | return "<if_none>"; 76 | } ... 288 | static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb, 289 | struct net_device *ndev) { ... 330 | brcmf_dbg(INFO, "%s: insufficient headroom (%d)\n", 331 | brcmf_ifname(ifp), head_delta); ... 336 | bphy_err(drvr, "%s: failed to expand headroom\n", 337 | brcmf_ifname(ifp)); For di->name, we expect di->name to be NUL-terminated based on its usage with format strings: | brcms_dbg_dma(di->core, | "%s: DMA64 tx doesn't have AE set\n", | di->name); Looking at its allocation we can see that it is already zero-allocated which means NUL-padding is not required: | di = kzalloc(sizeof(struct dma_info), GFP_ATOMIC); For wlc->modulecb[i].name, we expect each name in wlc->modulecb to be NUL-terminated based on their usage with strcmp(): | if (!strcmp(wlc->modulecb[i].name, name) && NUL-padding is not required as wlc is zero-allocated in: brcms_c_attach_malloc() -> | wlc = kzalloc(sizeof(struct brcms_c_info), GFP_ATOMIC); For all these cases, a suitable replacement is `strscpy` due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: Kalle Valo <kvalo@kernel.org> Link: https://lore.kernel.org/r/20231017-strncpy-drivers-net-wireless-broadcom-brcm80211-brcmfmac-cfg80211-c-v3-1-af780d74ae38@google.com
9d0d0a20