[PATCH v2] ath10k: add soft/hard firmware crash option to simulate_fw_crash

Jouni Malinen jkmalinen at gmail.com
Wed Mar 19 08:41:11 EDT 2014


On Wed, Mar 19, 2014 at 11:16 AM, Michal Kazior <michal.kazior at tieto.com> wrote:
>
> On 19 March 2014 10:06, Kalle Valo <kvalo at qca.qualcomm.com> 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



More information about the ath10k mailing list