Re: [PATCH v10 1/2] net: mhi: Enable Ethernet interface support

[Paolo Abeni <pabeni@redhat.com>]

On 4/9/26 8:08 AM, Vivek Pernamitta wrote:
> @@ -208,17 +235,20 @@ static void mhi_net_dl_callback(struct mhi_device *mhi_dev,
>  			skb = mhi_net_skb_agg(mhi_netdev, skb);
>  			mhi_netdev->skbagg_head = NULL;
>  		}
> -
> -		switch (skb->data[0] & 0xf0) {
> -		case 0x40:
> -			skb->protocol = htons(ETH_P_IP);
> -			break;
> -		case 0x60:
> -			skb->protocol = htons(ETH_P_IPV6);
> -			break;
> -		default:
> -			skb->protocol = htons(ETH_P_MAP);
> -			break;
> +		if (mhi_netdev->ndev->type == ARPHRD_ETHER) {
> +			skb->protocol = eth_type_trans(skb, mhi_netdev->ndev);

Sashiko says:

Is there a risk of an out-of-bounds read or kernel panic here if a malformed
or fragmented packet is received?
eth_type_trans() assumes the SKB has at least a 14-byte MAC header and calls
skb_pull_inline(). If the linear portion of the SKB is smaller than 14
bytes,
__skb_pull() will trigger a BUG_ON(skb->len < skb->data_len).
Should we call pskb_may_pull(skb, ETH_HLEN) before parsing the Ethernet
header?

> +		} else {
> +			switch (skb->data[0] & 0xf0) {
> +			case 0x40:
> +				skb->protocol = htons(ETH_P_IP);
> +				break;
> +			case 0x60:
> +				skb->protocol = htons(ETH_P_IPV6);
> +				break;
> +			default:
> +				skb->protocol = htons(ETH_P_MAP);
> +				break;
> +			}
>  		}
>  
>  		u64_stats_update_begin(&mhi_netdev->stats.rx_syncp);
> @@ -306,6 +336,9 @@ static int mhi_net_newlink(struct mhi_device *mhi_dev, struct net_device *ndev)
>  	struct mhi_net_dev *mhi_netdev;
>  	int err;
>  
> +	if (ndev->header_ops)
> +		eth_hw_addr_random(ndev);
> +
>  	mhi_netdev = netdev_priv(ndev);
>  
>  	dev_set_drvdata(&mhi_dev->dev, mhi_netdev);
> @@ -356,7 +389,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>  	int err;
>  
>  	ndev = alloc_netdev(sizeof(struct mhi_net_dev), info->netname,
> -			    NET_NAME_PREDICTABLE, mhi_net_setup);
> +			    NET_NAME_ENUM, info->ethernet_if ?
> +			    mhi_ethernet_setup : mhi_net_setup);

Sashiko says:

Does changing the name assignment type from NET_NAME_PREDICTABLE to
NET_NAME_ENUM break backwards compatibility for existing legacy interfaces?
NET_NAME_PREDICTABLE instructs userspace to leave the kernel-provided
name alone, while NET_NAME_ENUM signals that the interface is a generic
enumeration and should be renamed. Applying this to existing interfaces
like mhi_hwip0 and mhi_swip0 might cause them to be unexpectedly renamed
on boot, potentially breaking existing userspace network configurations.

please have a look at the full report:

https://sashiko.dev/#/patchset/20260409-vdev_b1_eth_b1_next-20260408-v10-0-6d44ca48f189%40oss.qualcomm.com

/P