On Sat, Oct 22, 2016 at 12:16:29AM +0200, Arnd Bergmann wrote: > On Friday, October 21, 2016 11:05:45 PM CEST Arnd Bergmann wrote: > > > > Can you explain why "dissector_uses_key(flow_dissector, > > FLOW_DISSECTOR_KEY_VLAN) && skb_vlan_tag_present(skb)" implies > > "eth_type_vlan(proto))"? > > > > If I add uninitialized_var() here, I would at least put that in > > a comment here. > > Found it now myself: if skb_vlan_tag_present(skb), then we don't > access 'vlan', otherwise we know it is initialized because > eth_type_vlan(proto) has to be true. > > > On a related note, I also don't see how > > "dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_VLAN)" > > implies that skb is non-NULL. I guess this is related to the > > first one. > > I'm still unsure about this one.
Only skb_flow_dissect_flow_keys_buf() calls this function with skb == NULL. It uses flow_keys_buf_dissector_keys which does not specify FLOW_DISSECTOR_KEY_VLAN, so the if statement is false. A similar assumption is made for FLOW_DISSECTOR_KEY_ETH_ADDRS higher up. > I also found something else that is suspicious: 'vlan' points > to the local _vlan variable, but that has gone out of scope > by the time we access the pointer, which doesn't seem safe. I see no harm in moving _vlan to the same scope as vlan.
