netdev - Re: [PATCH v1 net] af_unix: Add error handling in af_unix

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: <CAKgT0UcDsUOD_rhg9tzWVJkiL+ihwcuvZQ-_6ovRcwT79j6NKw@mail.gmail.com>
Date:   Wed, 14 Dec 2022 14:27:23 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Kuniyuki Iwashima <kuniyu@...zon.com>,
        "David S. Miller <davem@...emloft.net>, Eric Dumazet
        <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Denis V. Lunev
        <den@...nvz.org>, Kuniyuki Iwashima <kuni1840@...il.com>," 
        <netdev@...r.kernel.org>
Subject: Re: [PATCH v1 net] af_unix: Add error handling in af_unix_init().

On Wed, Dec 14, 2022 at 1:01 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed, 14 Dec 2022 11:18:05 -0800 Alexander H Duyck wrote:
> > So the "Fixes" tags for this are no good. The 2nd one is from the start
> > of using git for the kernel. As such I am suspecting that this isn't
> > fixing a patch that introduced an issue.
>
> We ask people to add the "start of history" tag when the issue goes all
> the way back.

The point I was getting at is that this issue doesn't really go all
the way back. Essentially sock_register could only fail if you were
registering a proto over 32 or one that was already registered. So
with 2.6.12-rc2 we should never see a failure without modifications to
the kernel as we only register PF_UNIX once and the other functions at
that time were void. This makes all the fixes tags suspect since the
patch doesn't resolve any issue with that code.

More likely candidates would have been:
Fixes: 94531cfcbe79 ("af_unix: Add unix_stream_proto for sockmap")
Fixes: 097e66c57845 ("[NET]: Make AF_UNIX per network namespace safe [v2]")

> > Since it doesn't really seem like this is fixing an issue other than
> > adding some additional exception handling. I would suggest getting rid
> > of the "Fixes" tags and just submitting this for net-next once the
> > window opens.
> >
> > The code itself looks fine, but I don't think it is really fixing much
> > either.
>
> We don't do gradation of fixes in netdev, if a function can fail not
> handling the exception is a bug. I'm not saying that you're wrong
> calling this out as highly theoretical, all I'm saying is that I for
> one do not have the mental stamina to try to establish and use more
> complex heuristics. We haven't had material reasons to introduce any.

My concern was that this is more of a refactor/cleanup posing as a
fix. I know I have been guilty of that once or twice myself trying to
squeeze a patch in during the merge window.