2021-03-26 20:59:09

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 1/3] mt76: mt7615: 0-terminate firmware log messages

Avoid including garbage from previous rx data in the printk messages

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index e565fc770a0f..0b8f5dc6b121 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -423,6 +423,7 @@ mt7615_mcu_rx_log_message(struct mt7615_dev *dev, struct sk_buff *skb)
break;
}

+ skb->data[skb->len] = 0;
wiphy_info(mt76_hw(dev)->wiphy, "%s: %s", type, data);
}

--
2.30.1


2021-03-26 21:10:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/3] mt76: mt7615: 0-terminate firmware log messages

>
> + skb->data[skb->len] = 0;
>   wiphy_info(mt76_hw(dev)->wiphy, "%s: %s", type, data);
>  }
>

Are you sure there's always enough space to write to the skb? Following
the code up I'm not even sure it's always a linear skb :-)

Might be safer/better to do

wiphy_info(..., "%s: %*s", type, skb->len - sizeof(*rxd),
data);

instead?

johannes


2021-03-26 21:54:54

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 1/3] mt76: mt7615: 0-terminate firmware log messages

On 2021-03-26 22:09, Johannes Berg wrote:
>>
>> + skb->data[skb->len] = 0;
>>   wiphy_info(mt76_hw(dev)->wiphy, "%s: %s", type, data);
>>  }
>>
>
> Are you sure there's always enough space to write to the skb? Following
> the code up I'm not even sure it's always a linear skb :-)
>
> Might be safer/better to do
>
> wiphy_info(..., "%s: %*s", type, skb->len - sizeof(*rxd),
> data);
>
> instead?
Maybe it does trust the firmware too much. I'll send a v2 based on your
suggestion.

Thanks,

- Felix