netdev - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 May 2019 09:25:18 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     maximmi@...lanox.com
Cc:     kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org,
        netdev@...r.kernel.org, leonro@...lanox.com
Subject: Re: [RFC] inet6_validate_link_af improvements

From: Maxim Mikityanskiy <maximmi@...lanox.com>
Date: Mon, 13 May 2019 15:05:28 +0000

> 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.

I think your patches should go in now.

And longer term for the other cases we should move to a prepare-->commit
model so that resources can be allocated in one pass and only commit the
result if all resources can be obtained together.