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 <[email protected]>
---
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(-)
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;
--
2.26.2
Takashi Iwai <[email protected]> writes:
> 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 <[email protected]>
I'm planning to queue this to v5.11. Should I add cc stable?
Luca, can I have your ack?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Tue, 12 Jan 2021 16:48:56 +0100,
Kalle Valo wrote:
>
> Takashi Iwai <[email protected]> writes:
>
> > 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 <[email protected]>
>
> I'm planning to queue this to v5.11. Should I add cc stable?
Yes, it hits 5.10.y.
> Luca, can I have your ack?
It'd be great if this fix goes in quickly.
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?
thanks,
Takashi
On Tue, 2021-01-12 at 17:02 +0100, Takashi Iwai wrote:
> On Tue, 12 Jan 2021 16:48:56 +0100,
> Kalle Valo wrote:
> >
> > Takashi Iwai <[email protected]> writes:
> >
> > > 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 <[email protected]>
> >
> > I'm planning to queue this to v5.11. Should I add cc stable?
>
> Yes, it hits 5.10.y.
>
> > Luca, can I have your ack?
>
> It'd be great if this fix goes in quickly.
Thanks for the fix!
Acked-by: Luca Coelho <[email protected]>
> 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, but the Fixes tag itself will almost always trigger the stable
people to take it anyway.
--
Cheers,
Luca.
"Coelho, Luciano" <[email protected]> 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/[email protected]
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 <[email protected]> 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: [email protected]
> Signed-off-by: Takashi Iwai <[email protected]>
> Acked-by: Luca Coelho <[email protected]>
Patch applied to wireless-drivers.git, thanks.
a6616bc9a0af iwlwifi: dbg: Don't touch the tlv data
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches