On Tue, 2014-03-11 at 18:15 -0400, Richard Guy Briggs wrote: > Steve, > > Subject says it all... > > Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO message? No... Given that userspace requests AUDIT_SIGNAL_INFO after it gets a signal, and that audit_sig_{uid,pid,...} get filled in when some task sent auditd that signal, the idea that the pid would be 0 doesn't make sense... (unless auditd requests AUDIT_SIGNAL_INFO without getting a signal, but that's just dumb)

On Tuesday, March 11, 2014 11:32:01 PM Richard Guy Briggs wrote: > On 14/03/11, Eric Paris wrote: > > On Tue, 2014-03-11 at 18:15 -0400, Richard Guy Briggs wrote: > > > Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO > > > message? > > > > No... > > > > Given that userspace requests AUDIT_SIGNAL_INFO after it gets a signal, > > and that audit_sig_{uid,pid,...} get filled in when some task sent > > auditd that signal, the idea that the pid would be 0 doesn't make > > sense... (unless auditd requests AUDIT_SIGNAL_INFO without getting a > > signal, but that's just dumb) > > The reason I ask is that it is initialized to -1, which I assume is no > more valid than zero in your interpretation above. pid=-1 has a special meaning for signals. But in terms of seeing it in a sigaction handler for siginfo, not possible. So its a good init value. If you look at sigaction(2), there is a si_code that indicates why the signal was sent. One of them is SI_KERNEL. So, its possible that the kernel decides to send a signal on certain occasions.

> I looked at converting audit_sig_pid from pid_t to struct pid *, but > then get_pid() would also be needed to protect that reference. A > put_pid() would need to be done once it is no longer needed, which could > be immediately after it is read in the AUDIT_SIGNAL_INFO message > preparation, assuming it would never need to be read again. If this > isn't the case, put_pid() could be called when audit_pid is nulled, but > if that message never comes, that struct pid is stuck with a stale > refcount. (That isn't an issue if it is init or systemd, but it is > still wrong.) > > This looks more and more like overkill and should probably leave > audit_sig_pid as pid_t. The code has been working good for a long time. I am wondering if the original intent was to make it general in case we decided to add more signals that we are interested in.

-Steve

On Wednesday, March 12, 2014 11:35:56 AM Richard Guy Briggs wrote: > > > I looked at converting audit_sig_pid from pid_t to struct pid *, but > > > then get_pid() would also be needed to protect that reference. A > > > put_pid() would need to be done once it is no longer needed, which could > > > be immediately after it is read in the AUDIT_SIGNAL_INFO message > > > preparation, assuming it would never need to be read again. If this > > > isn't the case, put_pid() could be called when audit_pid is nulled, but > > > if that message never comes, that struct pid is stuck with a stale > > > refcount. (That isn't an issue if it is init or systemd, but it is > > > still wrong.) > > > > > > This looks more and more like overkill and should probably leave > > > audit_sig_pid as pid_t. > > > > The code has been working good for a long time. I am wondering if the > > original intent was to make it general in case we decided to add more > > signals that we are interested in. > > Such as HUP to reread config or other possibilities? I think we started with sigterm. Then we needed sighup. Then needed usr1 & usr2. Somewhere along the way I think it was just decided to make it open ended in case more were needed later.

-Steve

On Tuesday, March 11, 2014 06:15:17 PM Richard Guy Briggs wrote: > Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO message? Well, pid=0 would be the kernel. So, its valid but unlikely. Offhand I don't know why the kernel might try sending a signal. Is this a problem?

-Steve

On Wed, 2014-03-12 at 08:22 -0400, Steve Grubb wrote: > On Tuesday, March 11, 2014 06:15:17 PM Richard Guy Briggs wrote: > > Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO message? > > Well, pid=0 would be the kernel. So, its valid but unlikely. Actually, signals from the kernel will never get recorded here... > Offhand I don't > know why the kernel might try sending a signal. Is this a problem? He's trying to figure out how to store this info in light of pid namespaces. right now, auditd can only live in the initial pid namespace, so can only get signals from processes in the initial pid namespace, so we can store it as a number always in the initial pid namespace. But if auditd were ever to not be in the initial pid namespace, not sure what to do....

> -Steve