From owner-svn-src-head@freebsd.org Wed May 3 19:03:29 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 720BCD575A4 for ; Wed, 3 May 2017 19:03:29 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1b.ore.mailhop.org (outbound1b.ore.mailhop.org [54.200.247.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 556DD1CDF for ; Wed, 3 May 2017 19:03:28 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-User: 3cce766d-3033-11e7-bfb5-0d159cd3c324 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 73.78.92.27 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [73.78.92.27]) by outbound1.ore.mailhop.org (Halon) with ESMTPSA id 3cce766d-3033-11e7-bfb5-0d159cd3c324; Wed, 03 May 2017 19:03:48 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id v43J3J2W001711; Wed, 3 May 2017 13:03:19 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1493838199.80042.14.camel@freebsd.org> Subject: Re: svn commit: r317755 - head/sbin/ifconfig From: Ian Lepore To: Ryan Stone , Alan Somers Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Date: Wed, 03 May 2017 13:03:19 -0600 In-Reply-To: References: <201705031721.v43HL2vS071819@repo.freebsd.org> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 May 2017 19:03:29 -0000 On Wed, 2017-05-03 at 14:07 -0400, Ryan Stone wrote: > On Wed, May 3, 2017 at 1:39 PM, Ryan Stone wrote: > > > > > > > > > On Wed, May 3, 2017 at 1:21 PM, Alan Somers wrote: > > > > > > > > Author: asomers > > > Date: Wed May  3 17:21:01 2017 > > > New Revision: 317755 > > > URL: https://svnweb.freebsd.org/changeset/base/317755 > > > > > > Log: > > >   Various Coverity fixes in ifconfig(8) > > > > > >   * Exit early if kldload(2) fails (1011259). This is the only change that > > >     affects ifconfig's behavior. > > > > > > > > Please revert this ASAP.  kldload is expected to fail for a number of > > benign reasons and this change is likely to prevent any network > > configuration from being applied to systems, breaking remote access. > > > > > It's been pointed out to me off-list that the situation is not quite as > dire as I had originally believed.  The ifconfig code in question already > searches to check if the module in question is loaded before calling > kldload.  However, there is at least one driver (mlx4_en) that does not > follow the "if_" kld module naming convention that this code depends > on, so this change will make it impossible to apply configuration to > mlx4_en interfaces.  Additionally, it is possible that other drivers use > the naming convention for their kld file but not for the module declared in > the C code, in which case this change would also break configuration of > those interfaces. > > jhb@ suggests that ifconfig should only attempt to load a module if the > interface doesn't already exist, by calling if_nametoindex to check for the > existence of the interface.  That seems to be a reasonable fix for me, but > in the interest of not breaking users' networking configuration > (potentially making it impossible to fix a remote machine), I'd recommend > that the part of the change that checks the return code from kldload() be > reverted while a fix for this issue is worked on. It should be noted that the existing code uses if_nametoindex() immediately after ifmaybeload() returns, and handles errors accordingly.  I.e., there wasn't really anything wrong with the code as originally written/structured. -- Ian