* Re: proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
[not found] <20131029230500.GC15011@mguzik.redhat.com>
@ 2013-11-11 17:21 ` Eric Paris
2013-11-11 18:56 ` Richard Guy Briggs
2013-11-11 19:20 ` Steve Grubb
0 siblings, 2 replies; 4+ messages in thread
From: Eric Paris @ 2013-11-11 17:21 UTC (permalink / raw)
To: Mateusz Guzik; +Cc: rgb, linux-audit
On Wed, 2013-10-30 at 00:05 +0100, Mateusz Guzik wrote:
> Hello,
>
> I wrote a trivial patch for what I believe is a subsystem you maintain.
>
> I'm sending it privately first to ensure it looks ok at has proper
> recipients (I'm new to linux world, sorry :>).
>
> 'To' would be: linux-audit@redhat.com
>
> The rest is:
>
> From: Mateusz Guzik <mguzik@redhat.com>
> Date: Tue, 29 Oct 2013 23:51:52 +0100
> Subject: [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
>
> kauditd_send_skb is called after audit_pid was checked to be non-zero.
>
> However, it can be set to 0 due to auditd exiting while kauditd_send_skb
> is still executed and this can result in a spurious warning about missing
> auditd.
>
> Re-check audit_pid before printing the message.
>
> Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
> Cc: Eric Paris <eparis@redhat.com>
> Cc: linux-kernel@vger.kernel.org
Acked-by: Eric Paris <eparis@redhat.com>
> ---
> kernel/audit.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 7b0e23a..a91a965 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -388,9 +388,11 @@ static void kauditd_send_skb(struct sk_buff *skb)
> err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
> if (err < 0) {
> BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
> - printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
> - audit_log_lost("auditd disappeared\n");
> - audit_pid = 0;
> + if (audit_pid) {
> + printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
> + audit_log_lost("auditd disappeared\n");
> + audit_pid = 0;
> + }
> /* we might get lucky and get this in the next auditd */
> audit_hold_skb(skb);
> } else
> --
> 1.8.3.1
>
> Is this ok?
>
> Thanks,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
2013-11-11 17:21 ` proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message Eric Paris
@ 2013-11-11 18:56 ` Richard Guy Briggs
2013-11-11 19:20 ` Steve Grubb
1 sibling, 0 replies; 4+ messages in thread
From: Richard Guy Briggs @ 2013-11-11 18:56 UTC (permalink / raw)
To: Eric Paris; +Cc: linux-audit, Mateusz Guzik
On Mon, Nov 11, 2013 at 12:21:33PM -0500, Eric Paris wrote:
> On Wed, 2013-10-30 at 00:05 +0100, Mateusz Guzik wrote:
> > Hello,
> >
> > I wrote a trivial patch for what I believe is a subsystem you maintain.
> >
> > I'm sending it privately first to ensure it looks ok at has proper
> > recipients (I'm new to linux world, sorry :>).
> >
> > 'To' would be: linux-audit@redhat.com
> >
> > The rest is:
> >
> > From: Mateusz Guzik <mguzik@redhat.com>
> > Date: Tue, 29 Oct 2013 23:51:52 +0100
> > Subject: [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
> >
> > kauditd_send_skb is called after audit_pid was checked to be non-zero.
> >
> > However, it can be set to 0 due to auditd exiting while kauditd_send_skb
> > is still executed and this can result in a spurious warning about missing
> > auditd.
> >
> > Re-check audit_pid before printing the message.
I'm ok with this. It can't hurt. I'll queue it.
> > Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
> > Cc: Eric Paris <eparis@redhat.com>
> > Cc: linux-kernel@vger.kernel.org
>
> Acked-by: Eric Paris <eparis@redhat.com>
>
> > kernel/audit.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 7b0e23a..a91a965 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -388,9 +388,11 @@ static void kauditd_send_skb(struct sk_buff *skb)
> > err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
> > if (err < 0) {
> > BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
> > - printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
> > - audit_log_lost("auditd disappeared\n");
> > - audit_pid = 0;
> > + if (audit_pid) {
> > + printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
> > + audit_log_lost("auditd disappeared\n");
> > + audit_pid = 0;
> > + }
> > /* we might get lucky and get this in the next auditd */
> > audit_hold_skb(skb);
> > } else
> > --
> > 1.8.3.1
> >
> > Is this ok?
> >
> > Thanks,
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
2013-11-11 17:21 ` proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message Eric Paris
2013-11-11 18:56 ` Richard Guy Briggs
@ 2013-11-11 19:20 ` Steve Grubb
2013-11-11 19:28 ` Eric Paris
1 sibling, 1 reply; 4+ messages in thread
From: Steve Grubb @ 2013-11-11 19:20 UTC (permalink / raw)
To: linux-audit; +Cc: rgb, Mateusz Guzik
On Monday, November 11, 2013 12:21:33 PM Eric Paris wrote:
> On Wed, 2013-10-30 at 00:05 +0100, Mateusz Guzik wrote:
> > Hello,
> >
> > I wrote a trivial patch for what I believe is a subsystem you maintain.
> >
> > I'm sending it privately first to ensure it looks ok at has proper
> > recipients (I'm new to linux world, sorry :>).
> >
> > 'To' would be: linux-audit@redhat.com
> >
> > The rest is:
> >
> > From: Mateusz Guzik <mguzik@redhat.com>
> > Date: Tue, 29 Oct 2013 23:51:52 +0100
> > Subject: [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
> >
> > kauditd_send_skb is called after audit_pid was checked to be non-zero.
> >
> > However, it can be set to 0 due to auditd exiting while kauditd_send_skb
> > is still executed and this can result in a spurious warning about missing
> > auditd.
Is it really spurious? Meaning an admin at a site making heavy use of the
audit subsystem would never want to know this? Are the events always put back
with no chance of loss?
-Steve
> > Re-check audit_pid before printing the message.
> >
> > Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
> > Cc: Eric Paris <eparis@redhat.com>
> > Cc: linux-kernel@vger.kernel.org
>
> Acked-by: Eric Paris <eparis@redhat.com>
>
> > ---
> >
> > kernel/audit.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 7b0e23a..a91a965 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -388,9 +388,11 @@ static void kauditd_send_skb(struct sk_buff *skb)
> >
> > err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
> > if (err < 0) {
> >
> > BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
> >
> > - printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n",
audit_pid);
> > - audit_log_lost("auditd disappeared\n");
> > - audit_pid = 0;
> > + if (audit_pid) {
> > + printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n",
audit_pid);
> > + audit_log_lost("auditd disappeared\n");
> > + audit_pid = 0;
> > + }
> >
> > /* we might get lucky and get this in the next auditd */
> > audit_hold_skb(skb);
> >
> > } else
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
2013-11-11 19:20 ` Steve Grubb
@ 2013-11-11 19:28 ` Eric Paris
0 siblings, 0 replies; 4+ messages in thread
From: Eric Paris @ 2013-11-11 19:28 UTC (permalink / raw)
To: Steve Grubb; +Cc: rgb, linux-audit, Mateusz Guzik
On Mon, 2013-11-11 at 14:20 -0500, Steve Grubb wrote:
> On Monday, November 11, 2013 12:21:33 PM Eric Paris wrote:
> > On Wed, 2013-10-30 at 00:05 +0100, Mateusz Guzik wrote:
> > > Hello,
> > >
> > > I wrote a trivial patch for what I believe is a subsystem you maintain.
> > >
> > > I'm sending it privately first to ensure it looks ok at has proper
> > > recipients (I'm new to linux world, sorry :>).
> > >
> > > 'To' would be: linux-audit@redhat.com
> > >
> > > The rest is:
> > >
> > > From: Mateusz Guzik <mguzik@redhat.com>
> > > Date: Tue, 29 Oct 2013 23:51:52 +0100
> > > Subject: [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message
> > >
> > > kauditd_send_skb is called after audit_pid was checked to be non-zero.
> > >
> > > However, it can be set to 0 due to auditd exiting while kauditd_send_skb
> > > is still executed and this can result in a spurious warning about missing
> > > auditd.
>
> Is it really spurious? Meaning an admin at a site making heavy use of the
> audit subsystem would never want to know this? Are the events always put back
> with no chance of loss?
>
> -Steve
auidt_pid will only get set to 0 in 2 ways. 1) auditd died, in which
case we will record the first lost message (because the pid won't be 0,
we set it to 0 after we record that message). 2) auditd/auditctl
explicitly set audit_pid to 0 (aka on shutdown) In which case, we
don't expect such a printk. It is normal that messages which race with
auditd shutting down may get 'lost'. If the message came 1 millisecond
later it would never have gotten on the queue since audit_pid would be
0.
-Eric
>
>
> > > Re-check audit_pid before printing the message.
> > >
> > > Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
> > > Cc: Eric Paris <eparis@redhat.com>
> > > Cc: linux-kernel@vger.kernel.org
> >
> > Acked-by: Eric Paris <eparis@redhat.com>
> >
> > > ---
> > >
> > > kernel/audit.c | 8 +++++---
> > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 7b0e23a..a91a965 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -388,9 +388,11 @@ static void kauditd_send_skb(struct sk_buff *skb)
> > >
> > > err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
> > > if (err < 0) {
> > >
> > > BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
> > >
> > > - printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n",
> audit_pid);
> > > - audit_log_lost("auditd disappeared\n");
> > > - audit_pid = 0;
> > > + if (audit_pid) {
> > > + printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n",
> audit_pid);
> > > + audit_log_lost("auditd disappeared\n");
> > > + audit_pid = 0;
> > > + }
> > >
> > > /* we might get lucky and get this in the next auditd */
> > > audit_hold_skb(skb);
> > >
> > > } else
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-11 19:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20131029230500.GC15011@mguzik.redhat.com>
2013-11-11 17:21 ` proposing [PATCH] audit: get rid of *NO* daemon at audit_pid=0 message Eric Paris
2013-11-11 18:56 ` Richard Guy Briggs
2013-11-11 19:20 ` Steve Grubb
2013-11-11 19:28 ` Eric Paris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.