On 19 March 2014 10:06, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Hi Dan,
>
> Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> writes:
>
>> From: Marek Puzyniak <marek.puzyniak@xxxxxxxxx>
>>
>> Command WMI_FORCE_FW_HANG_CMDID is not supported in firmware 10.1.
>> In order to have firmware crash simulation functionality also
>> in firmware 10.1 driver can force firmware crash by performing not allowed
>> operation. Driver can deliberately crash firmware when setting vdev param for
>> vdev id out of range. This patch introduces two keywords to simulate_fw_crash:
>>
>> 'soft' which will cause firmware crash that is recoverable
>> by warm firmware reset but supported only in main firmware.
>> 'hard' which will cause firmware crash recoverable by cold
>> firmware reset, this option works for both firmwares.
>>
>> Commands to trigger firmware soft/hard crash:
>>
>> echo 'soft' > /sys/kernel/debug/ieee80211/phyX/ath10k/simulate_fw_crash
>> echo 'hard' > /sys/kernel/debug/ieee80211/phyX/ath10k/simulate_fw_crash
>>
>> kvalo: use strncmp(), remove '\n' before checking the command and
>> document how buf is null terminated
>>
>> Signed-off-by: Marek Puzyniak <marek.puzyniak@xxxxxxxxx>
>> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx>
>
> [...]
>
>> @@ -479,14 +488,30 @@ static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
>> goto exit;
>> }
>>
>> - ath10k_info("simulating firmware crash\n");
>> + /* drop the possible '\n' from the end */
>> + if (buf[count - 1] == '\n') {
>> + buf[count - 1] = 0;
>> + count--;
>> + }
>>
>> - ret = ath10k_wmi_force_fw_hang(ar, WMI_FORCE_FW_HANG_ASSERT, 0);
>> - if (ret)
>> - ath10k_warn("failed to force fw hang (%d)\n", ret);
>> + if (!strncmp(buf, "soft", sizeof(buf))) {
>> + ath10k_info("simulating soft firmware crash\n");
>> + ret = ath10k_wmi_force_fw_hang(ar, WMI_FORCE_FW_HANG_ASSERT, 0);
>> + } else if (!strncmp(buf, "hard", sizeof(buf))) {
>> + ath10k_info("simulating hard firmware crash\n");
>> + ret = ath10k_wmi_vdev_set_param(ar, TARGET_NUM_VDEVS + 1,
>> + ar->wmi.vdev_param->rts_threshold, 0);
>> + } else {
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>
> 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.
Michał
--
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
