linux-kernel - Re: [GIT PULL] pidfd updates

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: <20230427-beobachten-flugzeit-300ecf4ae0a3@brauner>
Date:   Thu, 27 Apr 2023 11:40:22 +0200
From:   Christian Brauner <brauner@...nel.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] pidfd updates

On Thu, Apr 27, 2023 at 09:59:52AM +0100, Al Viro wrote:
> On Thu, Apr 27, 2023 at 10:33:38AM +0200, Christian Brauner wrote:
> 
> > File descriptor installation is not core functionality for drivers. It's
> > just something that they have to do and so it's not that people usually
> > put a lot of thought into it. So that's why I think an API has to be
> > dumb enough. A three call api may still be simpler to use than an overly
> > clever single call api.
> 
> Grep and you will see...  Seriously, for real mess take a look at e.g.
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c.  See those close_fd() calls in
> there?  That's completely wrong - you can't undo the insertion into
> descriptor table.

I mean, yes. There's literally a description how that's wrong in my pr
message...

> 
> I'm not suggesting that for the core kernel, but there's a plenty of
> drivers that do descriptor allocation.

That's undisputed. What I tried to say was that this is not their main
focus. The point is their design thinking is not centered on fd handling
that's just something they have to do and so they don't think about
cleanly handling for example installing an fd and the closing it again
in some failure path. And I'm not sure one should be just harshly
judging them. They're almost bound to get this wrong. You'd need to know
a bit about file descriptors to have this in mind. File descriptor
handling is subtle.

I mean, look at cachefiles_ondemand_get_fd()...