svn commit: r317755 - head/sbin/ifconfig

[Ryan Stone]

On Wed, May 3, 2017 at 1:39 PM, Ryan Stone <rysto32 at gmail.com> wrote:

>
>
> On Wed, May 3, 2017 at 1:21 PM, Alan Somers <asomers at 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_<drvr>" 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.