* [PATCH] brcmfmac: fix wrong strnchr usage
@ 2018-08-22 13:22 Rasmus Villemoes
2018-08-31 15:48 ` Kalle Valo
0 siblings, 1 reply; 2+ messages in thread
From: Rasmus Villemoes @ 2018-08-22 13:22 UTC (permalink / raw)
To: Kalle Valo, David S. Miller
Cc: netdev, linux-kernel, linux-wireless, Rasmus Villemoes
strnchr takes arguments in the order of its name: string, max bytes to
read, character to search for. Here we're passing '\n' aka 10 as the
buffer size, and searching for sizeof(buf) aka BRCMF_DCMD_SMLEN aka
256 (aka '\0', since it's implicitly converted to char) within those 10
bytes.
Just interchanging the last two arguments would still leave a bug,
because if we've been successful once, there are not sizeof(buf)
characters left after the new value of p.
Since clmver is immediately afterwards passed as a %s argument, I assume
that it is actually a properly nul-terminated string. For that case, we
have strreplace().
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Incidentally, I found this because I was looking at our strnchr
implementation and noticed a corner case: strchr() is specified by C and
POSIX standards to consider the trailing '\0' to be part of the string,
so strchr(foo, '\0') must be equivalent to "foo +
strlen(foo)". strnchr() is not specified by anyone. Our implementation
returns NULL as soon as count is depleted or hitting a '\0' byte, so
since we were here looking for a '\0' byte, the first strnchr() call was
guaranteed to return NULL.
Had strnchr() instead returned a pointer to the '\0' if such was found
within the first count (in this case 10) bytes, we'd be overwriting that
with a ' ', and continue until no '\0' was found within the next 10
bytes.
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index cd3651069d0c..94044a7a6021 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -296,9 +296,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
/* Replace all newline/linefeed characters with space
* character
*/
- ptr = clmver;
- while ((ptr = strnchr(ptr, '\n', sizeof(buf))) != NULL)
- *ptr = ' ';
+ strreplace(clmver, '\n', ' ');
brcmf_dbg(INFO, "CLM version = %s\n", clmver);
}
--
2.16.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] brcmfmac: fix wrong strnchr usage
2018-08-22 13:22 [PATCH] brcmfmac: fix wrong strnchr usage Rasmus Villemoes
@ 2018-08-31 15:48 ` Kalle Valo
0 siblings, 0 replies; 2+ messages in thread
From: Kalle Valo @ 2018-08-31 15:48 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: David S. Miller, netdev, linux-kernel, linux-wireless, Rasmus Villemoes
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> strnchr takes arguments in the order of its name: string, max bytes to
> read, character to search for. Here we're passing '\n' aka 10 as the
> buffer size, and searching for sizeof(buf) aka BRCMF_DCMD_SMLEN aka
> 256 (aka '\0', since it's implicitly converted to char) within those 10
> bytes.
>
> Just interchanging the last two arguments would still leave a bug,
> because if we've been successful once, there are not sizeof(buf)
> characters left after the new value of p.
>
> Since clmver is immediately afterwards passed as a %s argument, I assume
> that it is actually a properly nul-terminated string. For that case, we
> have strreplace().
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Patch applied to wireless-drivers-next.git, thanks.
cb18e2e9ec71 brcmfmac: fix wrong strnchr usage
--
https://patchwork.kernel.org/patch/10572951/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-08-31 15:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-22 13:22 [PATCH] brcmfmac: fix wrong strnchr usage Rasmus Villemoes
2018-08-31 15:48 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).