Date: Wed, 03 May 2017 13:03:19 -0600 From: Ian Lepore <ian@freebsd.org> To: Ryan Stone <rysto32@gmail.com>, Alan Somers <asomers@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r317755 - head/sbin/ifconfig Message-ID: <1493838199.80042.14.camel@freebsd.org> In-Reply-To: <CAFMmRNyUXZz==Xdh8fqbw7BqYCHig%2BXAEOj1dxcJ2p0H6_qWeA@mail.gmail.com> References: <201705031721.v43HL2vS071819@repo.freebsd.org> <CAFMmRNyHc7qgeidDTZss%2Bo=56Uc-Xgyu_voj4ybHt=mEJa6c%2Bg@mail.gmail.com> <CAFMmRNyUXZz==Xdh8fqbw7BqYCHig%2BXAEOj1dxcJ2p0H6_qWeA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 2017-05-03 at 14:07 -0400, Ryan Stone wrote: > On Wed, May 3, 2017 at 1:39 PM, Ryan Stone <rysto32@gmail.com> wrote: > > > > > > > > > On Wed, May 3, 2017 at 1:21 PM, Alan Somers <asomers@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1493838199.80042.14.camel>