On Friday, February 26, 2016 17:56:13 Sven Eckelmann wrote:
> --- a/net/batman-adv/soft-interface.c
> +++ b/net/batman-adv/soft-interface.c
> @@ -408,11 +408,17 @@ void batadv_interface_rx(struct net_device
> *soft_iface, */
>  	nf_reset(skb);
> 
> +	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
> +		goto dropped;
> +
>  	vid = batadv_get_vid(skb, 0);

batadv_get_vid() also calls pskb_may_pull() and checks for VLAN_ETH_HLEN 
length. Isn't that sufficient ?

On a related note - a few lines before your check you'll find:

/* check if enough space is available for pulling, and pull */
if (!pskb_may_pull(skb, hdr_size))

In its current form that check is useless because batadv_recv_unicast_packet() 
already calls batadv_check_unicast_packet() which does the same 
pskb_may_pull(skb, hdr_size). Am I overlooking something ?


>  	switch (ntohs(ethhdr->h_proto)) {
>  	case ETH_P_8021Q:
> +		if (!pskb_may_pull(skb, VLAN_ETH_HLEN))
> +			goto dropped;

Shouldn't this memory access be covered by the earlier check inside 
batadv_get_vid() ?


>  	/* skb->dev & skb->pkt_type are set here */
> -	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
> -		goto dropped;

Agreed that this seems unnecessary.

Cheers,
Marek