| Message ID | 20210112132449.22243-2-tiwai@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | iwlwifi: Fix a crash at loading | expand |
"Coelho, Luciano" <luciano.coelho@intel.com> writes: >> BTW, I thought network people don't want to have Cc-to-stable in the >> patch, so I didn't put it by myself. Is this rule still valid? > > In the wireless side of network, we've always used Cc stable when > needed Yeah, we handle stable patches differently from the main network tree. > but the Fixes tag itself will almost always trigger the stable > people to take it anyway. BTW, this is now clarified in the documentation: https://lkml.kernel.org/r/20210113163315.1331064-1-lee.jones@linaro.org So cc stable should be added even if there's already a Fixes tag. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Takashi Iwai <tiwai@suse.de> wrote: > The commit ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device > memory") added a termination of name string just to be sure, and this > seems causing a regression, a GPF triggered at firmware loading. > Basically we shouldn't modify the firmware data that may be provided > as read-only. > > This patch drops the code that caused the regression and keep the tlv > data as is. > > Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory") > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1180344 > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210733 > Cc: stable@vger.kernel.org > Signed-off-by: Takashi Iwai <tiwai@suse.de> > Acked-by: Luca Coelho <luciano.coelho@intel.com> Patch applied to wireless-drivers.git, thanks. a6616bc9a0af iwlwifi: dbg: Don't touch the tlv data -- https://patchwork.kernel.org/project/linux-wireless/patch/20210112132449.22243-2-tiwai@suse.de/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c index a654147d3cd6..a80a35a7740f 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c @@ -180,13 +180,6 @@ static int iwl_dbg_tlv_alloc_region(struct iwl_trans *trans, if (le32_to_cpu(tlv->length) < sizeof(*reg)) return -EINVAL; - /* For safe using a string from FW make sure we have a - * null terminator - */ - reg->name[IWL_FW_INI_MAX_NAME - 1] = 0; - - IWL_DEBUG_FW(trans, "WRT: parsing region: %s\n", reg->name); - if (id >= IWL_FW_INI_MAX_REGION_ID) { IWL_ERR(trans, "WRT: Invalid region id %u\n", id); return -EINVAL;
The commit ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory") added a termination of name string just to be sure, and this seems causing a regression, a GPF triggered at firmware loading. Basically we shouldn't modify the firmware data that may be provided as read-only. This patch drops the code that caused the regression and keep the tlv data as is. Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory") BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1180344 BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210733 Signed-off-by: Takashi Iwai <tiwai@suse.de> --- As an alternative fix, the debug print could be with the printf length specifier to limit the size to IWL_FW_INIT_MAX_NAME, as found in the bugzilla entries above, too. I chose a simpler (partial) revert here instead. drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 7 ------- 1 file changed, 7 deletions(-)