On 02/20, Christian Brauner wrote: > > On Tue, Feb 20, 2024 at 12:00:12PM +0100, Oleg Nesterov wrote: > > > > Perhaps we can kill the "task_pid(current) != pid" check and just return > > EPERM if "kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL", I don't think > > anobody needs pidfd_send_send_signal() to signal yourself. See below. > > Yeah. You have my ack in advance > > > + /* Currently unused. */ > > > + if (info) > > > + return -EINVAL; > > > > Well, to me this looks like the unnecessary restriction... And why? > > Because right now we aren't sure that it's used Yes, but... > and we aren't sure what use-cases are there. the same use-cases as for rt_sigqueueinfo() ? Christian, I won't really argue but I still disagree. Let me first repeat once again, I do not know what people do with pidfd and pidfd_send_signal() in particular, so I won't be surprised if this change won't cause any regression report. But at the same time, I can easily imagine the following scenario: a userspace programmer tries to use pidfd_send_signal(info != NULL), gets -EINVAL, decides it can't/shouldn't work, and switches to sigqueueinfo() without any report to lkml. > Yes, absolutely. That was always the plan. See appended patch I put on top. > I put you as author since you did spot this. Let me know if you don't > want that. Ah. Thanks Christian. I am fine either way, whatever is more convenient for you. But just in case, I won't mind at all if you simply fold this minor fix into your PIDFD_SEND_PROCESS_GROUP patch, I certainly don't care about the "From" tag ;) A really, really minor/cosmetic nit below, feel free to ignore: > - if ((task_pid(current) != pid) && > + if (((task_pid(current) != pid) || type > PIDTYPE_TGID) && we can remove the unnecessary parens around "task_pid(current) != pid" or add the extra parens aroung "type > PIDTYPE_TGID". I mean, the 1st operand of "&&" is (task_pid(current) != pid) || type > PIDTYPE_TGID and this looks a bit inconsistent to me. Oleg.
