On Wed, 23 Sep 2020 21:16:27 -0700 Florian Fainelli wrote:
> While we should always make sure that we specify a valid VLAN protocol
> to vlan_proto_idx(), killing the machine when an invalid value is
> specified is too harsh and not helpful for debugging. All callers are
> capable of dealing with an error returned by vlan_proto_idx() so check
> the index value and propagate it accordingly.
> 
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>

Perhaps it's heresy but I wonder if the error checking is worth it 
or we'd be better off WARNing and assuming normal Q tag.. unlikely
someone is getting it wrong now given the BUG().

> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index d4bcfd8f95bf..6c08de1116c1 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -57,6 +57,9 @@ static int vlan_group_prealloc_vid(struct vlan_group *vg,
>       ASSERT_RTNL();
>  
>       pidx  = vlan_proto_idx(vlan_proto);
> +     if (pidx < 0)
> +             return -EINVAL;
> +
>       vidx  = vlan_id / VLAN_GROUP_ARRAY_PART_LEN;
>       array = vg->vlan_devices_arrays[pidx][vidx];
>       if (array != NULL)
> diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
> index bb7ec1a3915d..143e9c12dbd6 100644
> --- a/net/8021q/vlan.h
> +++ b/net/8021q/vlan.h
> @@ -44,8 +44,8 @@ static inline unsigned int vlan_proto_idx(__be16 proto)

adjust return type

>       case htons(ETH_P_8021AD):
>               return VLAN_PROTO_8021AD;
>       default:
> -             BUG();
> -             return 0;
> +             WARN(1, "invalid VLAN protocol: 0x%04x\n", htons(proto));

ntohs()

> +             return -EINVAL;
>       }
>  }

Reply via email to