Messages in this thread | | | Subject | Re: [PATCH] fcntl: make F_GETOWN(EX) return 0 on dead owner task | From | Pavel Tikhomirov <> | Date | Mon, 8 Feb 2021 15:57:15 +0300 |
| |
On 2/8/21 3:31 PM, Jeff Layton wrote: > On Thu, 2021-02-04 at 01:17 +0300, Cyrill Gorcunov wrote: >> On Thu, Feb 04, 2021 at 12:35:42AM +0300, Pavel Tikhomirov wrote: >>> >>> AFAICS if pid is held only by 1) fowner refcount and by 2) single process >>> (without threads, group and session for simplicity), on process exit we go >>> through: >>> >>> do_exit >>> exit_notify >>> release_task >>> __exit_signal >>> __unhash_process >>> detach_pid >>> __change_pid >>> free_pid >>> idr_remove >>> >>> So pid is removed from idr, and after that alloc_pid can reuse pid numbers >>> even if old pid structure is still alive and is still held by fowner. >> ... >>> Hope this answers your question, Thanks! >> >> Yeah, indeed, thanks! So the change is sane still I'm >> a bit worried about backward compatibility, gimme some >> time I'll try to refresh my memory first, in a couple >> of days or weekend (though here are a number of experienced >> developers CC'ed maybe they reply even faster). > > I always find it helpful to refer to the POSIX spec [1] for this sort of > thing. In this case, it says: > > F_GETOWN > If fildes refers to a socket, get the process ID or process group ID > specified to receive SIGURG signals when out-of-band data is available. > Positive values shall indicate a process ID; negative values, other than > -1, shall indicate a process group ID; the value zero shall indicate > that no SIGURG signals are to be sent. If fildes does not refer to a > socket, the results are unspecified. > > In the event that the PID is reused, the kernel won't send signals to > the replacement task, correct?
Correct. Looks like only places to send signal to owner are send_sigio() and send_sigurg() (at least nobody else dereferences fown->pid_type). And in both places we lookup for task to send signal to with pid_task() or do_each_pid_task() (similar to what I do in patch) and will not find any task if pid was reused. Thus no signal would be sent.
> Assuming that's the case, then this patch > looks fine to me too. I'll plan to pick it for linux-next later today, > and we can hopefully get this into v5.12. > > [1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html#tag_16_122 >
Thanks for finding it!
-- Best regards, Tikhomirov Pavel Software Developer, Virtuozzo.
| |