On Wed, Jun 21, 2023 at 07:05:12AM -0400, Jeff Layton wrote: > On Wed, 2023-06-21 at 15:42 +0500, stsp wrote: > > 21.06.2023 15:35, Jeff Layton пишет: > > > I don't think we can change this at this point. > > > > > > The bottom line (again) is that OFD locks are owned by the file > > > descriptor (much like with flock()), and since file descriptors can be > > > shared across multiple process it's impossible to say that some single > > > process owns it. > > What's the problem with 2 owners? > > Can't you get one of them, rather than > > meaningless -1? > > Compare this situation with read locks. > > They can overlap, so when you get an > > info about a read lock (except for the > > new F_UNLCK case), you get the info > > about *some* of the locks in that range. > > In the case of multiple owners, you > > likewise get the info about about some > > owner. If you iteratively send them a > > "please release this lock" message > > (eg in a form of SIGKILL), then you > > traverse all, and end up with the > > lock-free area. > > Is there really any problem here? > > Yes. Ambiguous answers are worse than none at all. I agree. A few minor observations: SCM_RIGHTS have already been mentioned multiple times. But I'm not sure it's been mentioned explicitly but that trivially means it's possible to send an fd to a completely separate thread-group, then kill off the sending thread-group by killing their thread-group leader. Bad enough as the identifier is now useless. But it also means that at some later point that pid can be recycled. So using that pid for killing sprees is usually a bad idea and might cause killing a completely separate thread-group. pidfd_getfd() can be used to retrieve a file descriptor from another process; like an inverse dup. Opens up similar problems as SCM_RIGHTS. Expressing ownership doesn't make sense as this is an inherently shared concept as Jeff mentioned. So best case is to try and track the creator similar to SO_PEERCRED for unix sockets. In any case, this is all made worse by the fact that struct file_lock stashes raw pid numbers. That means that the kernel itself can't even detect whether the creator of that lock has died and the pid possibly been recycled. In essence, it risk reporting as owner a random process that just happens to have the same pid number as the creator that might've died. Even for unix sockets SO_PEERCRED and SCM_CREDS are inherently racy even though they stash a struct pid instead of a raw pid number and that makes them problematic in a lot of scenario which is why they will support pidfds with v6.5. The situation could probably be improved by stashing a struct pid in struct file_lock then you could at least somewhat reasonably track the creator as the kernel would be able to detect whether the creating process has already been reaped (not exited). For example, I did this for pidfds where it reports -1 if the process the pidfd refers to has been reaped. Btw, is there a way to limit the number of file locks a user might create? Is that implicitly bound to the maximum number of fds and files a caller may create because I saw RLIMIT_LOCKS but I didn't see it checked anywhere.
