On 9/24/2020 4:46 PM, Jakub Kicinski wrote:
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 <[email protected]>
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().
This showed up while working with Vladimir on another patch set where we
were able to submit packets with an incorrect skb->vlan_proto submitted
via the DSA stacking model. It should not happen, but arguably crashing
the kernel was not helping either.
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
Ah yes, indeed.
case htons(ETH_P_8021AD):
return VLAN_PROTO_8021AD;
default:
- BUG();
- return 0;
+ WARN(1, "invalid VLAN protocol: 0x%04x\n", htons(proto));
ntohs()
OK, there is also a variable name pdix instead of pidx, I will resend
shortly, thanks!
--
Florian
