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; > } > }