| Message ID | b0d233386e059bccb59f18f69afb79a7806e5ded.1694507226.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 486e6ca6b48d68d7fefc99e15cc1865e2210d893 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethernet: mtk_wed: check update_wo_rx_stats in mtk_wed_update_rx_stats() | expand |
On Tue, Sep 12, 2023 at 10:28:00AM +0200, Lorenzo Bianconi wrote: > Check if update_wo_rx_stats function pointer is properly set in > mtk_wed_update_rx_stats routine before accessing it. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> Hi Lorenzo, I'm a little curious about this. Is there a condition where it is not set but accessed, which would presumably be a bug that warrants a fixes tag and targeting at 'net'? Or can it not occur, in which case this check is perhaps not needed? Or something else? > --- > drivers/net/ethernet/mediatek/mtk_wed_mcu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > index 071ed3dea860..72bcdaed12a9 100644 > --- a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > +++ b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > @@ -68,6 +68,9 @@ mtk_wed_update_rx_stats(struct mtk_wed_device *wed, struct sk_buff *skb) > struct mtk_wed_wo_rx_stats *stats; > int i; > > + if (!wed->wlan.update_wo_rx_stats) > + return; > + > if (count * sizeof(*stats) > skb->len - sizeof(u32)) > return; > > -- > 2.41.0 > >
> On Tue, Sep 12, 2023 at 10:28:00AM +0200, Lorenzo Bianconi wrote: > > Check if update_wo_rx_stats function pointer is properly set in > > mtk_wed_update_rx_stats routine before accessing it. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > Hi Lorenzo, Hi Simon, > > I'm a little curious about this. > > Is there a condition where it is not set but accessed, > which would presumably be a bug that warrants a fixes tag and > targeting at 'net'? > > Or can it not occur, in which case this check is perhaps not needed? nope, so far Wireless Ethernet Dispatches (WED) is supported just by mt7915 that sets update_wo_rx_stats callback. Howerver, I am currently working on WED support for mt7996 where we do not have this callback available at the moment. Regards, Lorenzo > > Or something else? > > > --- > > drivers/net/ethernet/mediatek/mtk_wed_mcu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > > index 071ed3dea860..72bcdaed12a9 100644 > > --- a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > > +++ b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c > > @@ -68,6 +68,9 @@ mtk_wed_update_rx_stats(struct mtk_wed_device *wed, struct sk_buff *skb) > > struct mtk_wed_wo_rx_stats *stats; > > int i; > > > > + if (!wed->wlan.update_wo_rx_stats) > > + return; > > + > > if (count * sizeof(*stats) > skb->len - sizeof(u32)) > > return; > > > > -- > > 2.41.0 > > > >
On Wed, Sep 13, 2023 at 01:47:49PM +0200, Lorenzo Bianconi wrote: > > On Tue, Sep 12, 2023 at 10:28:00AM +0200, Lorenzo Bianconi wrote: > > > Check if update_wo_rx_stats function pointer is properly set in > > > mtk_wed_update_rx_stats routine before accessing it. > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > Hi Lorenzo, > > Hi Simon, > > > > > I'm a little curious about this. > > > > Is there a condition where it is not set but accessed, > > which would presumably be a bug that warrants a fixes tag and > > targeting at 'net'? > > > > Or can it not occur, in which case this check is perhaps not needed? > > nope, so far Wireless Ethernet Dispatches (WED) is supported just by mt7915 > that sets update_wo_rx_stats callback. Howerver, I am currently working on WED > support for mt7996 where we do not have this callback available at the moment. Thanks Lorenzo, Understood. In that case the patch looks good to me. Reviewed-by: Simon Horman <horms@kernel.org>
Hello: This patch was applied to netdev/net-next.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 12 Sep 2023 10:28:00 +0200 you wrote: > Check if update_wo_rx_stats function pointer is properly set in > mtk_wed_update_rx_stats routine before accessing it. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > drivers/net/ethernet/mediatek/mtk_wed_mcu.c | 3 +++ > 1 file changed, 3 insertions(+) Here is the summary with links: - [net-next] net: ethernet: mtk_wed: check update_wo_rx_stats in mtk_wed_update_rx_stats() https://git.kernel.org/netdev/net-next/c/486e6ca6b48d You are awesome, thank you!
diff --git a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c index 071ed3dea860..72bcdaed12a9 100644 --- a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c +++ b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c @@ -68,6 +68,9 @@ mtk_wed_update_rx_stats(struct mtk_wed_device *wed, struct sk_buff *skb) struct mtk_wed_wo_rx_stats *stats; int i; + if (!wed->wlan.update_wo_rx_stats) + return; + if (count * sizeof(*stats) > skb->len - sizeof(u32)) return;
Check if update_wo_rx_stats function pointer is properly set in mtk_wed_update_rx_stats routine before accessing it. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- drivers/net/ethernet/mediatek/mtk_wed_mcu.c | 3 +++ 1 file changed, 3 insertions(+)