From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758567AbcJYILT (ORCPT ); Tue, 25 Oct 2016 04:11:19 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:38648 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753431AbcJYILP (ORCPT ); Tue, 25 Oct 2016 04:11:15 -0400 Date: Tue, 25 Oct 2016 10:11:12 +0200 From: Jiri Pirko To: Arnd Bergmann Cc: "David S. Miller" , Eric Garver , Hadar Hen Zion , Jiri Pirko , Alexander Duyck , Tom Herbert , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 net] flow_dissector: fix vlan tag handling Message-ID: <20161025081112.GB1672@nanopsycho.orion> References: <20161024214058.1896091-1-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024214058.1896091-1-arnd@arndb.de> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mon, Oct 24, 2016 at 11:40:30PM CEST, arnd@arndb.de wrote: >gcc warns about an uninitialized pointer dereference in the vlan >priority handling: > >net/core/flow_dissector.c: In function '__skb_flow_dissect': >net/core/flow_dissector.c:281:61: error: 'vlan' may be used uninitialized in this function [-Werror=maybe-uninitialized] > >As pointed out by Jiri Pirko, the variable is never actually used >without being initialized first as the only way it end up uninitialized >is with skb_vlan_tag_present(skb)==true, and that means it does not >get accessed. > >However, the warning hints at some related issues that I'm addressing >here: > >- the second check for the vlan tag is different from the first one > that tests the skb for being NULL first, causing both the warning > and a possible NULL pointer dereference that was not entirely fixed. >- The same patch that introduced the NULL pointer check dropped an > earlier optimization that skipped the repeated check of the > protocol type >- The local '_vlan' variable is referenced through the 'vlan' pointer > but the variable has gone out of scope by the time that it is > accessed, causing undefined behavior > >Caching the result of the 'skb && skb_vlan_tag_present(skb)' check >in a local variable allows the compiler to further optimize the >later check. With those changes, the warning also disappears. > >Fixes: 3805a938a6c2 ("flow_dissector: Check skb for VLAN only if skb specified.") >Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci") >Signed-off-by: Arnd Bergmann Acked-by: Jiri Pirko