[v1] net: mediatek: add null pointer check for hardware offloading

[PATCH] net: mediatek: add null pointer check for hardware offloading

Posted by Sebastian Roland Wolf 1 month, 1 week ago

From: Sebastian Roland Wolf <srw@root533.premium-rootserver.net>

Add a null pointer check to prevent kernel crashes when hardware
offloading is active on MediaTek devices.

In some edge cases, the ethernet pointer or its associated netdev
element can be NULL. Checking these pointers before access is
mandatory to avoid segmentation faults and kernel oops.

This improves the robustness of the validation check for mtk_eth
ingress devices introduced in commit 73cfd947dbdb ("net: mediatek:
add support for ingress traffic offloading").

Fixes: 73cfd947dbdb ("net: mediatek: add support for ingress traffic offloading")
net: mediatek: Add null pointer check to prevent crashes with active hardware offloading.

Signed-off-by: Sebastian Roland Wolf <Sebastian.Wolf@pace-systems.de>
---
 drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
index e9bd32741983..6900ac87e1e9 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
@@ -270,7 +270,8 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
 		flow_rule_match_meta(rule, &match);
 		if (mtk_is_netsys_v2_or_greater(eth)) {
 			idev = __dev_get_by_index(&init_net, match.key->ingress_ifindex);
-			if (idev && idev->netdev_ops == eth->netdev[0]->netdev_ops) {
+			if (idev && eth && eth->netdev[0] &&
+			    idev->netdev_ops == eth->netdev[0]->netdev_ops) {
 				struct mtk_mac *mac = netdev_priv(idev);
 
 				if (WARN_ON(mac->ppe_idx >= eth->soc->ppe_num))
-- 
2.51.0

Re: [PATCH] net: mediatek: add null pointer check for hardware offloading

Posted by Vadim Fedorenko 1 month ago

On 31/12/2025 22:52, Sebastian Roland Wolf wrote:
> From: Sebastian Roland Wolf <srw@root533.premium-rootserver.net>
> 
> Add a null pointer check to prevent kernel crashes when hardware
> offloading is active on MediaTek devices.
> 
> In some edge cases, the ethernet pointer or its associated netdev
> element can be NULL. Checking these pointers before access is
> mandatory to avoid segmentation faults and kernel oops.
> 
> This improves the robustness of the validation check for mtk_eth
> ingress devices introduced in commit 73cfd947dbdb ("net: mediatek:
> add support for ingress traffic offloading").
> 
> Fixes: 73cfd947dbdb ("net: mediatek: add support for ingress traffic offloading")
> net: mediatek: Add null pointer check to prevent crashes with active hardware offloading.
> 
> Signed-off-by: Sebastian Roland Wolf <Sebastian.Wolf@pace-systems.de>
> ---
>   drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index e9bd32741983..6900ac87e1e9 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -270,7 +270,8 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
>   		flow_rule_match_meta(rule, &match);
>   		if (mtk_is_netsys_v2_or_greater(eth)) {

The code dereferences eth here ...

>   			idev = __dev_get_by_index(&init_net, match.key->ingress_ifindex);
> -			if (idev && idev->netdev_ops == eth->netdev[0]->netdev_ops) {
> +			if (idev && eth && eth->netdev[0] &&

... but it is checked a couple of lines after.

Even more, the function starts with providing rhahstable to lookup
cookie. I'm really doubt eth can be NULL.
At the same time lack of eth->netdev[0] looks like a design problem,
because according to the code there might be up to 3 netdev devices
registered for ppe.

I'm not familiar with the code, but it would be better to have a splat
of crash to check what was exactly missing, and drgn can help you find
if there were other netdevs available at the moment of crash.

> +			    idev->netdev_ops == eth->netdev[0]->netdev_ops) {
>   				struct mtk_mac *mac = netdev_priv(idev);
>   
>   				if (WARN_ON(mac->ppe_idx >= eth->soc->ppe_num))

Re: [PATCH] net: mediatek: add null pointer check for hardware offloading

Posted by Sebastian Wolf 1 month ago

Hi Vadim,
"Vadim Fedorenko" vadim.fedorenko@linux.dev – 2. Januar 2026 12:20
> On 31/12/2025 22:52, Sebastian Roland Wolf wrote:
> > From: Sebastian Roland Wolf <srw@root533.premium-rootserver.net>
> > 
> > Add a null pointer check to prevent kernel crashes when hardware
> > offloading is active on MediaTek devices.
> > 
> > In some edge cases, the ethernet pointer or its associated netdev
> > element can be NULL. Checking these pointers before access is
> > mandatory to avoid segmentation faults and kernel oops.
> > 
> > This improves the robustness of the validation check for mtk_eth
> > ingress devices introduced in commit 73cfd947dbdb ("net: mediatek:
> > add support for ingress traffic offloading").
> > 
> > Fixes: 73cfd947dbdb ("net: mediatek: add support for ingress traffic offloading")
> > net: mediatek: Add null pointer check to prevent crashes with active hardware offloading.
> > 
> > Signed-off-by: Sebastian Roland Wolf <Sebastian.Wolf@pace-systems.de>
> > ---
> > drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> > index e9bd32741983..6900ac87e1e9 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> > @@ -270,7 +270,8 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
> > flow_rule_match_meta(rule, &match);
> > if (mtk_is_netsys_v2_or_greater(eth)) {
> 
> The code dereferences eth here ...
> 
> > idev = __dev_get_by_index(&init_net, match.key->ingress_ifindex);
> > - if (idev && idev->netdev_ops == eth->netdev[0]->netdev_ops) {
> > + if (idev && eth && eth->netdev[0] &&
> 
> ... but it is checked a couple of lines after.
> 
You are right that 'eth' is already dereferenced above, so checking it here is redundant. I will remove the 'eth' check in a V2 of this patch. (As this is first patch ever I hope I do it right)
> Even more, the function starts with providing rhahstable to lookup
> cookie. I'm really doubt eth can be NULL.
> At the same time lack of eth->netdev[0] looks like a design problem,
> because according to the code there might be up to 3 netdev devices
> registered for ppe.
While it might point to a deeper design issue, the check for 'eth->netdev[0]' is necessary to prevent the immediate kernel oops I am seeing. Forcing this check prevents the crash, which is the lesser evil compared to a complete system failure, even if hardware offloading might not function correctly in that specific state.
> 
> I'm not familiar with the code, but it would be better to have a splat
> of crash to check what was exactly missing, and drgn can help you find
> if there were other netdevs available at the moment of crash.
Unfortunately, I am not a regular kernel developer and have no experience with drgn. Furthermore, I am testing this on a production device which limits my ability to perform deep interactive debugging or long-term crash analysis.
> 
> > + idev->netdev_ops == eth->netdev[0]->netdev_ops) {
> > struct mtk_mac *mac = netdev_priv(idev);
> > 
> > if (WARN_ON(mac->ppe_idx >= eth->soc->ppe_num))
> 
>
I will send a V2 shortly, focusing on the 'eth->netdev[0]' check while removing the redundant 'eth' check.
Best regards,
Sebastian