From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sabrina Dubroca Subject: Re: [PATCH net] ipmr: fix error path when mr_table_alloc fails Date: Tue, 5 Jun 2018 12:17:16 +0200 Message-ID: <20180605101716.GA17412@bistromath.localdomain> References: <20180604.172514.1940145189918116408.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: netdev@vger.kernel.org, edumazet@google.com, nikolay@cumulusnetworks.com, yuvalm@mellanox.com, ivecera@redhat.com To: David Miller Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:52090 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751890AbeFEKRP (ORCPT ); Tue, 5 Jun 2018 06:17:15 -0400 Content-Disposition: inline In-Reply-To: <20180604.172514.1940145189918116408.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: 2018-06-04, 17:25:14 -0400, David Miller wrote: > From: Sabrina Dubroca > Date: Mon, 4 Jun 2018 13:55:54 +0200 > > > commit 0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table") > > refactored ipmr_new_table, so that it now returns NULL when > > mr_table_alloc fails. Unfortunately, all callers of ipmr_new_table > > expect an ERR_PTR. commit 66fb33254f45 ("ipmr: properly check > > rhltable_init() return value") followed suit. > > > > This can result in NULL deref, when ipmr_rules_exit calls > > ipmr_free_table with NULL net->ipv4.mrt in the > > !CONFIG_IP_MROUTE_MULTIPLE_TABLES version. > > > > This patch makes mr_table_alloc return errors, and changes > > ip6mr_new_table and its callers to return/expect error pointers as > > well. It also removes the version of mr_table_alloc defined under > > !CONFIG_IP_MROUTE_COMMON, since it is never used. > > > > Fixes: 0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table") > > Fixes: 66fb33254f45 ("ipmr: properly check rhltable_init() return value") > > Signed-off-by: Sabrina Dubroca > > This adds a new warning with gcc-8.1.1 on Fedora 28 > > CC [M] net/ipv6/ip6mr.o > In file included from ./arch/x86/include/asm/current.h:5, > from ./include/linux/sched.h:12, > from ./include/linux/uaccess.h:5, > from net/ipv6/ip6mr.c:19: > net/ipv6/ip6mr.c: In function ‘ip6_mroute_setsockopt’: > ./include/linux/compiler.h:177:26: warning: ‘mrt’ may be used uninitialized in this function [-Wmaybe-uninitialized] > case 8: *(__u64 *)res = *(volatile __u64 *)p; break; \ > ^ > net/ipv6/ip6mr.c:1752:20: note: ‘mrt’ was declared here > struct mr_table *mrt; > ^~~ grmbl, CONFIG_UBSAN disables -Wmaybe-uninitialized. I'll prepare a v2, sorry. -- Sabrina