netdev - Re: [net] Revert "net: core: maybe return -EEXIST in __dev_alloc

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]

Message-ID: <87efnpnf8b.fsf@rasmusvillemoes.dk>
Date:   Thu, 21 Dec 2017 00:37:08 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Michael Ellerman <michael@...cordia.ellerman.id.au>
Cc:     "Johannes Berg" <johannes@...solutions.net>,
        "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "Jouni Malinen" <j@...fi>,
        "Johannes Berg" <johannes.berg@...el.com>,
        "linuxppc-dev\@lists.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [net] Revert "net: core: maybe return -EEXIST in __dev_alloc_name"

On Tue, Dec 19 2017, Michael Ellerman <michael@...cordia.ellerman.id.au> wrote:

> Hi Johannes,
>
>> From: Johannes Berg <johannes.berg@...el.com>
>> 
>> This reverts commit d6f295e9def0; some userspace (in the case
>
> This revert seems to have broken networking on one of my powerpc
> machines, according to git bisect.
>
> The symptom is DHCP fails and I don't get a link, I didn't dig any
> further than that. I can if it's helpful.
>
> I think the problem is that 87c320e51519 ("net: core: dev_get_valid_name
> is now the same as dev_alloc_name_ns") only makes sense while
> d6f295e9def0 remains in the tree.

I'm sorry about all of this, I really didn't think there would be such
consequences of changing an errno return. Indeed, d6f29 was preparation
for unifying the two functions that do the exact same thing (and how we
ever got into that situation is somewhat unclear), except for
their behaviour in the case the requested name already exists. So one of
the two interfaces had to change its return value, and as I wrote, I
thought EEXIST was the saner choice when an explicit name (no %d) had
been requested.

> ie. before the entire series, dev_get_valid_name() would return EEXIST,
> and that was retained when 87c320e51519 was merged, but now that
> d6f295e9def0 has been reverted dev_get_valid_name() is returning ENFILE.
>
> I can get the network up again if I also revert 87c320e51519 ("net:
> core: dev_get_valid_name is now the same as dev_alloc_name_ns"), or with
> the gross patch below.

I don't think changing -ENFILE to -EEXIST would be right either, since
dev_get_valid_name() used to be able to return both (-EEXIST in the case
where there's no %d, -ENFILE in the case where we end up calling
dev_alloc_name_ns()). If anything, we could do the check for the old
-EEXIST condition first, and then call dev_alloc_name_ns(). But I'm also
fine with reverting.

Again, sorry :(

Rasmus