netdev - [RFC] inet6_validate_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190513150513.26872-1-maximmi@mellanox.com>
Date:   Mon, 13 May 2019 15:05:28 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Leon Romanovsky <leonro@...lanox.com>,
        Maxim Mikityanskiy <maximmi@...lanox.com>
Subject: [RFC] inet6_validate_link_af improvements

A recent bug in systemd [1] triggered the following kernel warning:

  A link change request failed with some changes committed already.
  Interface eth1 may have been left with an inconsistent configuration,
  please check.

do_setlink() performs multiple configuration updates, and if any of them
fails, do_setlink() has no way to revert the previous ones. It is also
not easy to validate everything in advance and perform a non-failing
update then. However, do_setlink() has some basic validation that can be
extended at least this case. IMO, it's better to fail before doing any
changes than to perform a partial configuration update.

This RFC contains two patches that move some checks to the validation
stage (inet6_validate_link_af() function). Only one of the patches (if
any) should be applied. Patch 1 only checks the presence of at least one
parameter, and patch 2 also moves the validation for addrgenmode that is
currently part of inet6_set_link_af(). Of course, there are still many
ways how do_setlink() can fail and perform a partial update, but IMO
it's better to prevent at least some cases that we can.

Please express your opinions on this fix: do we need it, do we want to
validate as much as possible (patch 2) or only basic stuff like the
presence of parameters (patch 1)? I'm looking forward to hearing the
feedback.

Thanks,
Max

[1]: https://github.com/systemd/systemd/issues/12504

-- 
2.19.1