On Wed, Mar 19, 2014 at 11:16 AM, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
>
> On 19 March 2014 10:06, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> > Fengguan's buildbot got warnings here and I assume they are coming from
> > smatch:
> >
> > drivers/net/wireless/ath/ath10k/debug.c:500 ath10k_write_simulate_fw_crash() error: strncmp() '"hard"' too small (5 vs 32)
> > drivers/net/wireless/ath/ath10k/debug.c:497 ath10k_write_simulate_fw_crash() error: strncmp() '"soft"' too small (5 vs 32)
> >
> > I wanted to use strncmp() instead of strcmp(), but I'm not sure what to
> > do here. In my opinion it's guaranteed that the string "hard" is null
> > terminated, so it shouldn't matter even if strlen("soft") (5) is less
> > than sizeof(buf) (32), right? Or am I missing something here?
>
> Hmm.. strncmp() compares *at most* n chars. The above means you can
> overflow the const char[] "hard" and "soft" if `buf` is longer than
> those. strncmp() must be passed the smallest length of either
> argument.
No you cannot. strncmp() will stop at '\0' from either buffer. If buf
is nul terminated, I see no point in using strncmp here (i.e., strcmp
should be used instead). If buf is not nul terminated, the length to
strncmp() must be the correct length of data in that buf, not
sizeof(buf). In either case, the current version here is incorrect
(even if it could not result in a buffer read overflow in practice).
- Jouni
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
