Linux-audit Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-05  9:26 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <563B1409.3030803@debian.org>



Le 05/11/15 09:32, Laurent Bigonville a écrit :
> Le 05/11/15 04:23, Steve Grubb a écrit :
>> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but 
>> I also
>> did not get an error message in syslog. So, I don't know what to make 
>> of it.
>> (And for the record, I have a bz open saying that USER_AVC is the 
>> wrong event
>> type. They are blaming libselinux but I blame them for not using
>> AUDIT_USER_MAC_POLICY_LOAD.)
> The audit code in dbus has been refactored a bit in the version 
> present F23 and debian unstable, so it might be related to this that.
>
> Do you still have the number of that bz bug?

BTW, systemd is also apparently sending a USER_AVC event when the policy 
is reloaded.

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-05  8:32 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <2867240.eZb4Ly0uub@x2>

Le 05/11/15 04:23, Steve Grubb a écrit :
> On Tuesday, November 03, 2015 09:48:31 PM Laurent Bigonville wrote:
>> Le 03/11/15 21:08, Richard Guy Briggs a écrit :
>>> On 15/11/03, Steve Grubb wrote:
>>>> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
>>>>> I'm running in permissive mode.
>>>>>
>>>>> I'm seeing a netlink open to the audit:
>>>>>
>>>>> dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT
>>>>>
>>>>> Apparently audit_send() returns -1
>>>> Since its -1, that would be an EPERM. No idea where this is coming from
>>>> if you have CAP_AUDIT_WRITE. I use pscap to check that.
>>> Are you in a container of any kind or any non-init USER namespace?  I
>>> can't see it being denied otherwise assuming it is only trying to send
>>> AUDIT_USER_* class messages.  (This assumes upstream kernel.)
>> No, I initially saw this on my laptop and then tested on F23 in kvm.
> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but I also
> did not get an error message in syslog. So, I don't know what to make of it.
> (And for the record, I have a bz open saying that USER_AVC is the wrong event
> type. They are blaming libselinux but I blame them for not using
> AUDIT_USER_MAC_POLICY_LOAD.)
The audit code in dbus has been refactored a bit in the version present 
F23 and debian unstable, so it might be related to this that.

Do you still have the number of that bz bug?

Cheers,

Laurent Bigonville

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Steve Grubb @ 2015-11-05  3:23 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <56391D9F.3080301@debian.org>

On Tuesday, November 03, 2015 09:48:31 PM Laurent Bigonville wrote:
> Le 03/11/15 21:08, Richard Guy Briggs a écrit :
> > On 15/11/03, Steve Grubb wrote:
> >> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
> >>> I'm running in permissive mode.
> >>> 
> >>> I'm seeing a netlink open to the audit:
> >>> 
> >>> dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT
> >>> 
> >>> Apparently audit_send() returns -1
> >> 
> >> Since its -1, that would be an EPERM. No idea where this is coming from
> >> if you have CAP_AUDIT_WRITE. I use pscap to check that.
> > 
> > Are you in a container of any kind or any non-init USER namespace?  I
> > can't see it being denied otherwise assuming it is only trying to send
> > AUDIT_USER_* class messages.  (This assumes upstream kernel.)
> 
> No, I initially saw this on my laptop and then tested on F23 in kvm.

I tested this on Fedora 22 and did not get a USER_AVC from dbus, but I also 
did not get an error message in syslog. So, I don't know what to make of it. 
(And for the record, I have a bz open saying that USER_AVC is the wrong event 
type. They are blaming libselinux but I blame them for not using 
AUDIT_USER_MAC_POLICY_LOAD.)

-Steve

> > I guess I have to ask which kernel too, since changes to NET and PID
> > namespaces are somewhat recent and Debian tends on the side of
> > conservative to be stable.
> 
> I'm under debian unstable and the kernel I'm running is 4.2
> 
> >>> I've been to reproduce this on F23 as well.
> >> 
> >> I have not played around with that yet.
> > 
> > What kernel is that?
> 
> 4.2 too apparently.
> 
> Cheers,
> 
> Laurent Bigonville
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply

* Re: [RFC PATCH 1/7] audit: don't needlessly reset valid wait time
From: Richard Guy Briggs @ 2015-11-05  3:13 UTC (permalink / raw)
  To: Paul Moore; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <1597895.v7jZTs1jqP@sifl>

On 15/11/04, Paul Moore wrote:
> On Thursday, October 22, 2015 02:53:14 PM Richard Guy Briggs wrote:
> > After auditd has recovered from an overflowed queue, the first process
> > that doesn't use reserves to make it through the queue checks should
> > reset the audit backlog wait time to the configured value.  After that,
> > there is no need to keep resetting it.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/audit.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index a72ad37..daefd81 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1403,7 +1403,7 @@ struct audit_buffer *audit_log_start(struct
> > audit_context *ctx, gfp_t gfp_mask, return NULL;
> >  	}
> > 
> > -	if (!reserve)
> > +	if (!reserve && !audit_backlog_wait_time)
> >  		audit_backlog_wait_time = audit_backlog_wait_time_master;
> > 
> >  	ab = audit_buffer_alloc(ctx, gfp_mask, type);
> 
> This looks fine to me, I'm going to add it to audit#next-queue.
> 
> Also, can you think of a good reason why "audit_backlog_wait_overflow" exists?  
> I'm going to replace it with the simple "audit_backlog_wait_time = 0;" unless 
> you can think of a solid reason not to do so.  It seems much more obvious and 
> readable to me.

That goes back to ac4cec44, DWMW, July 2005.  Best answer I can come up
with is that it labels magic values and puts them up front at the top of
the file.  I'd suggest instead replacing it with a macro.  I don't have
an significant objection to just assigning zero where you suggest.

> paul moore

- 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

* Re: [RFC PATCH 2/7] audit: include auditd's threads in audit_log_start() wait exception
From: Paul Moore @ 2015-11-04 23:08 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <96c9dc3a967e093da911e7b1b222c190572db987.1445539473.git.rgb@redhat.com>

On Thursday, October 22, 2015 02:53:15 PM Richard Guy Briggs wrote:
> Should auditd spawn threads, allow all members of its thread group to
> use the audit_backlog_limit reserves to bypass the queue limits too.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Looks good, applied to audit#next-queue.
 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index daefd81..3917aad 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1375,7 +1375,7 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, return NULL;
> 
>  	if (gfp_mask & __GFP_WAIT) {
> -		if (audit_pid && audit_pid == current->pid)
> +		if (audit_pid && audit_pid == current->tgid)
>  			gfp_mask &= ~__GFP_WAIT;
>  		else
>  			reserve = 0;

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* [RFC PATCH] audit: remove audit_backlog_wait_overflow
From: Paul Moore @ 2015-11-04 23:04 UTC (permalink / raw)
  To: linux-audit

It seems much more obvious and readable to simply use "0".

Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 kernel/audit.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 5a3ae37..6b4ae65 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -110,7 +110,6 @@ static u32	audit_backlog_limit = 64;
 #define AUDIT_BACKLOG_WAIT_TIME (60 * HZ)
 static u32	audit_backlog_wait_time_master = AUDIT_BACKLOG_WAIT_TIME;
 static u32	audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;
-static u32	audit_backlog_wait_overflow = 0;
 
 /* The identity of the user shutting down the audit system. */
 kuid_t		audit_sig_uid = INVALID_UID;
@@ -1395,7 +1394,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 				skb_queue_len(&audit_skb_queue),
 				audit_backlog_limit);
 		audit_log_lost("backlog limit exceeded");
-		audit_backlog_wait_time = audit_backlog_wait_overflow;
+		audit_backlog_wait_time = 0;
 		wake_up(&audit_backlog_wait);
 		return NULL;
 	}

^ permalink raw reply related

* Re: [RFC PATCH 1/7] audit: don't needlessly reset valid wait time
From: Paul Moore @ 2015-11-04 23:03 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <2b43c5d14e97945ea743f2e93e766834c93404fd.1445539473.git.rgb@redhat.com>

On Thursday, October 22, 2015 02:53:14 PM Richard Guy Briggs wrote:
> After auditd has recovered from an overflowed queue, the first process
> that doesn't use reserves to make it through the queue checks should
> reset the audit backlog wait time to the configured value.  After that,
> there is no need to keep resetting it.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index a72ad37..daefd81 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1403,7 +1403,7 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, return NULL;
>  	}
> 
> -	if (!reserve)
> +	if (!reserve && !audit_backlog_wait_time)
>  		audit_backlog_wait_time = audit_backlog_wait_time_master;
> 
>  	ab = audit_buffer_alloc(ctx, gfp_mask, type);

This looks fine to me, I'm going to add it to audit#next-queue.

Also, can you think of a good reason why "audit_backlog_wait_overflow" exists?  
I'm going to replace it with the simple "audit_backlog_wait_time = 0;" unless 
you can think of a solid reason not to do so.  It seems much more obvious and 
readable to me.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [GIT PULL] Audit patches for 4.4
From: Paul Moore @ 2015-11-04 13:38 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <4657963.SteroFqOP3@sifl>

On Wednesday, November 04, 2015 08:34:12 AM Paul Moore wrote:
> Hi Linus,
> 
> Seven audit patches for 4.4, but really only one of any significant value,
> the remainder are trivial cleanups that are described well enough in the
> patch descriptions.  The one significant patch is an attempt to make
> communication between the kernel's audit subsystem and the userspace audit
> daemon a bit more robust by retrying on certain transient error conditions.
>  All in all, it's a pretty small set of patches this time around with just
> fixes and cleanups, please pull for 4.4.

Just a FYI for the linux-audit folks - I rebased the audit#upstream on v4.3 
for this pull request, you'll see this trickle down to audit#next once the 
merge window closes and I rotate the branches.  Sorry for any problems this 
may cause.

-- 
paul moore
security @ redhat

^ permalink raw reply

* [GIT PULL] Audit patches for 4.4
From: Paul Moore @ 2015-11-04 13:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-audit, linux-kernel

Hi Linus,

Seven audit patches for 4.4, but really only one of any significant value, the 
remainder are trivial cleanups that are described well enough in the patch 
descriptions.  The one significant patch is an attempt to make communication 
between the kernel's audit subsystem and the userspace audit daemon a bit more 
robust by retrying on certain transient error conditions.  All in all, it's a 
pretty small set of patches this time around with just fixes and cleanups, 
please pull for 4.4.

Thanks,
-Paul

---
The following changes since commit 6a13feb9c82803e2b815eca72fa7a9f5561d7861:

  Linux 4.3 (2015-11-01 16:05:25 -0800)

are available in the git repository at:

  git://git.infradead.org/users/pcmoore/audit upstream

for you to fetch changes up to 233a68667cf4c134d07ef7e22bdd77786b5c7360:

  audit: make audit_log_common_recv_msg() a void function
        (2015-11-04 08:23:52 -0500)

----------------------------------------------------------------
Paul Moore (1):
      audit: make audit_log_common_recv_msg() a void function

Richard Guy Briggs (1):
      audit: try harder to send to auditd upon netlink failure

Saurabh Sengar (1):
      audit: removing unused variable

Scott Matheina (1):
      audit: fix comment block whitespace

Yaowei Bai (3):
      audit: audit_dummy_context can be boolean
      audit: audit_string_contains_control can be boolean
      audit: audit_tree_match can be boolean

 include/linux/audit.h |  8 ++++----
 kernel/audit.c        | 42 ++++++++++++++++++++++++++++--------------
 kernel/audit.h        |  2 +-
 kernel/audit_tree.c   |  6 +++---
 kernel/auditfilter.c  | 14 +++++++-------
 5 files changed, 43 insertions(+), 29 deletions(-)

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-03 20:48 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <20151103200811.GG1422@madcap2.tricolour.ca>

Le 03/11/15 21:08, Richard Guy Briggs a écrit :
> On 15/11/03, Steve Grubb wrote:
>> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
>>>
>>> I'm running in permissive mode.
>>>
>>> I'm seeing a netlink open to the audit:
>>>
>>> dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT
>>>
>>> Apparently audit_send() returns -1
>> Since its -1, that would be an EPERM. No idea where this is coming from if you
>> have CAP_AUDIT_WRITE. I use pscap to check that.
> Are you in a container of any kind or any non-init USER namespace?  I
> can't see it being denied otherwise assuming it is only trying to send
> AUDIT_USER_* class messages.  (This assumes upstream kernel.)

No, I initially saw this on my laptop and then tested on F23 in kvm.

> I guess I have to ask which kernel too, since changes to NET and PID
> namespaces are somewhat recent and Debian tends on the side of
> conservative to be stable.

I'm under debian unstable and the kernel I'm running is 4.2

>
>>> I've been to reproduce this on F23 as well.
>> I have not played around with that yet.
> What kernel is that?

4.2 too apparently.

Cheers,

Laurent Bigonville

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Richard Guy Briggs @ 2015-11-03 20:08 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <4171588.zUAisyrW99@x2>

On 15/11/03, Steve Grubb wrote:
> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
> > Le 03/11/15 17:28, Steve Grubb a écrit :
> > > On Tuesday, November 03, 2015 05:05:55 PM Laurent Bigonville wrote:
> > >> Hi,
> > >> 
> > >> With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system
> > >> dbus daemon is complaining with the following message:
> > >> 
> > >> nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC
> > >> avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon"
> > >> sauid=102 hostname=? addr=? terminal=?
> > >> 
> > >> This is the system dbus daemon running as "messagebus":
> > >> 
> > >> message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11
> > >> /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile
> > >> --systemd-activation
> > >> 
> > >> Looking at the capabilities:
> > >> 
> > >> $ sudo getpcaps 1057
> > >> Capabilities for `1057': = cap_audit_write+ep
> > >> 
> > >> All other user_avc seems to be properly logged in audit.
> > >> 
> > >> An idea?
> > > 
> > > I'd patch it to syslog errno and other information to locate the syscall
> > > that's failing. Did socket fail? Did the send fail? Does it work in
> > > permissive mode?
> > 
> > I'm running in permissive mode.
> > 
> > I'm seeing a netlink open to the audit:
> > 
> > dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT
> > 
> > Apparently audit_send() returns -1
> 
> Since its -1, that would be an EPERM. No idea where this is coming from if you 
> have CAP_AUDIT_WRITE. I use pscap to check that.

Are you in a container of any kind or any non-init USER namespace?  I
can't see it being denied otherwise assuming it is only trying to send
AUDIT_USER_* class messages.  (This assumes upstream kernel.)

I guess I have to ask which kernel too, since changes to NET and PID
namespaces are somewhat recent and Debian tends on the side of
conservative to be stable.

> > I've been to reproduce this on F23 as well.
> 
> I have not played around with that yet. 

What kernel is that?

> > BTW if I'm trying to compile audit with gcc optimization disabled (-O0)
> > I get:
> > 
> > libtool: link: gcc -D_GNU_SOURCE -g -O0 -fstack-protector-strong
> > -Wformat -Werror=format-security -Wl,-z -Wl,relro -Wl,--as-needed -o
> > .libs/auvirt auvirt.o auvirt-list.o ausearch-time.o  -L../../auparse
> > /<<PKGBUILDDIR>>/debian/build/auparse/.libs/libauparse.so
> > auvirt.o: In function `process_machine_id_event':
> > /<<PKGBUILDDIR>>/debian/build/tools/auvirt/../../../../tools/auvirt/auvirt.c
> > :484: undefined reference to `copy_str'
> 
> Thanks. I see a similar report with a patch from yoctoproject.org whatever 
> that is. I don't recall seeing the patch sent here. They list it as a C99 
> compiler change in semantics for inline functions. I have fixed this differently 
> in the upstream code as commit #1132

Yocto is a framework for developing distributions for embedded devices.

> https://fedorahosted.org/audit/changeset/1132
> 
> Thanks,
> -Steve

- 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

* Re: Audit Framework and namespaces
From: Richard Guy Briggs @ 2015-11-03 19:44 UTC (permalink / raw)
  To: Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <B41870ED03633F4092CDF476119204DF54632FEB@G9W0758.americas.hpqcorp.net>

On 15/11/03, Gulland, Scott A wrote:
> Does the audit framework work with linux namespaces?

The quick answer is "Some".

I am not aware of any restrictions on running audit services in MNT, UTS
or IPC namespaces.  The upstream kernel has support for running auditd
in any network namespace.  Additionally, processes with CAP_AUDIT_WRITE
(generally to send AUDIT_USER_* class messages) can send from any PID
namespace, but auditd is not permitted to run anywhere other than in the
initial PID namespace.  There is no support for any audit services from
any USER namespace other than initial due to serious concerns with
security, policy and experience still accumulating in that area.  There
are expectations that this latter will be supported in the future, but
that needs planning, execution and thorough testing.

I hope this helps answer your question.  I note you didn't ask about
audit working in containers, which is a harder question to answer
clearly due to the definition of "container".  The last point made in
the paragraph above will get us closer to supporting audit services in
Linux containers.

> Scott Gulland

- 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

* Re: SELinux policy reload cannot be sent to audit system
From: Steve Grubb @ 2015-11-03 19:33 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <5638EAE7.1070506@debian.org>

On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
> Le 03/11/15 17:28, Steve Grubb a écrit :
> > On Tuesday, November 03, 2015 05:05:55 PM Laurent Bigonville wrote:
> >> Hi,
> >> 
> >> With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system
> >> dbus daemon is complaining with the following message:
> >> 
> >> nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC
> >> avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon"
> >> sauid=102 hostname=? addr=? terminal=?
> >> 
> >> This is the system dbus daemon running as "messagebus":
> >> 
> >> message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11
> >> /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile
> >> --systemd-activation
> >> 
> >> Looking at the capabilities:
> >> 
> >> $ sudo getpcaps 1057
> >> Capabilities for `1057': = cap_audit_write+ep
> >> 
> >> All other user_avc seems to be properly logged in audit.
> >> 
> >> An idea?
> > 
> > I'd patch it to syslog errno and other information to locate the syscall
> > that's failing. Did socket fail? Did the send fail? Does it work in
> > permissive mode?
> 
> I'm running in permissive mode.
> 
> I'm seeing a netlink open to the audit:
> 
> dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT
> 
> Apparently audit_send() returns -1

Since its -1, that would be an EPERM. No idea where this is coming from if you 
have CAP_AUDIT_WRITE. I use pscap to check that.


> I've been to reproduce this on F23 as well.

I have not played around with that yet. 


> BTW if I'm trying to compile audit with gcc optimization disabled (-O0)
> I get:
> 
> libtool: link: gcc -D_GNU_SOURCE -g -O0 -fstack-protector-strong
> -Wformat -Werror=format-security -Wl,-z -Wl,relro -Wl,--as-needed -o
> .libs/auvirt auvirt.o auvirt-list.o ausearch-time.o  -L../../auparse
> /<<PKGBUILDDIR>>/debian/build/auparse/.libs/libauparse.so
> auvirt.o: In function `process_machine_id_event':
> /<<PKGBUILDDIR>>/debian/build/tools/auvirt/../../../../tools/auvirt/auvirt.c
> :484: undefined reference to `copy_str'

Thanks. I see a similar report with a patch from yoctoproject.org whatever 
that is. I don't recall seeing the patch sent here. They list it as a C99 
compiler change in semantics for inline functions. I have fixed this differently 
in the upstream code as commit #1132

https://fedorahosted.org/audit/changeset/1132

Thanks,
-Steve

^ permalink raw reply

* Re: Audit Framework and namespaces
From: Paul Moore @ 2015-11-03 19:04 UTC (permalink / raw)
  To: Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <B41870ED03633F4092CDF476119204DF54632FEB@G9W0758.americas.hpqcorp.net>

On Tue, Nov 3, 2015 at 12:34 PM, Gulland, Scott A <scott.gulland@hpe.com> wrote:
> Does the audit framework work with linux namespaces?

I'm sorry, you'll have to be more specific than that; what exactly are
you interested in with respect to audit and namespaces?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Audit Framework and namespaces
From: Gulland, Scott A @ 2015-11-03 17:34 UTC (permalink / raw)
  To: linux-audit@redhat.com, Gulland, Scott A


[-- Attachment #1.1.1: Type: text/plain, Size: 222 bytes --]

Does the audit framework work with linux namespaces?

Scott Gulland
HP Networking, CEB R&D

916.785.1497
Hewlett-Packard Company
8000 Foothills Blvd; MS - 5505
Roseville, CA 95747
USA
[hp]<http://www.hp.com/>


[-- Attachment #1.1.2: Type: text/html, Size: 2734 bytes --]

[-- Attachment #1.2: image001.png --]
[-- Type: image/png, Size: 5197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-03 17:12 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <1758315.3fUBHW9xxQ@x2>

Le 03/11/15 17:28, Steve Grubb a écrit :
> On Tuesday, November 03, 2015 05:05:55 PM Laurent Bigonville wrote:
>> Hi,
>>
>> With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system
>> dbus daemon is complaining with the following message:
>>
>> nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC
>> avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon"
>> sauid=102 hostname=? addr=? terminal=?
>>
>> This is the system dbus daemon running as "messagebus":
>>
>> message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11
>> /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile
>> --systemd-activation
>>
>> Looking at the capabilities:
>>
>> $ sudo getpcaps 1057
>> Capabilities for `1057': = cap_audit_write+ep
>>
>> All other user_avc seems to be properly logged in audit.
>>
>> An idea?
> I'd patch it to syslog errno and other information to locate the syscall
> that's failing. Did socket fail? Did the send fail? Does it work in permissive
> mode?
I'm running in permissive mode.

I'm seeing a netlink open to the audit:

dbus-daem 1057 messagebus    7u  netlink 0t0  15248 AUDIT

Apparently audit_send() returns -1

I've been to reproduce this on F23 as well.

BTW if I'm trying to compile audit with gcc optimization disabled (-O0) 
I get:

libtool: link: gcc -D_GNU_SOURCE -g -O0 -fstack-protector-strong 
-Wformat -Werror=format-security -Wl,-z -Wl,relro -Wl,--as-needed -o 
.libs/auvirt auvirt.o auvirt-list.o ausearch-time.o  -L../../auparse 
/<<PKGBUILDDIR>>/debian/build/auparse/.libs/libauparse.so
auvirt.o: In function `process_machine_id_event':
/<<PKGBUILDDIR>>/debian/build/tools/auvirt/../../../../tools/auvirt/auvirt.c:484: 
undefined reference to `copy_str'

Cheers,

Laurent Bigonville

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Paul Moore @ 2015-11-03 16:38 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <1758315.3fUBHW9xxQ@x2>

On Tue, Nov 3, 2015 at 11:28 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Tuesday, November 03, 2015 05:05:55 PM Laurent Bigonville wrote:
>> Hi,
>>
>> With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system
>> dbus daemon is complaining with the following message:
>>
>> nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC
>> avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon"
>> sauid=102 hostname=? addr=? terminal=?
>>
>> This is the system dbus daemon running as "messagebus":
>>
>> message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11
>> /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile
>> --systemd-activation
>>
>> Looking at the capabilities:
>>
>> $ sudo getpcaps 1057
>> Capabilities for `1057': = cap_audit_write+ep
>>
>> All other user_avc seems to be properly logged in audit.
>>
>> An idea?
>
> I'd patch it to syslog errno and other information to locate the syscall
> that's failing. Did socket fail? Did the send fail? Does it work in permissive
> mode?

I would also verify that your loaded SELinux policy is not blocking
the CAP_AUDIT_WRITE capability or the netlink_audit_socket:nlmsg_relay
permission.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: SELinux policy reload cannot be sent to audit system
From: Steve Grubb @ 2015-11-03 16:28 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <5638DB63.7010204@debian.org>

On Tuesday, November 03, 2015 05:05:55 PM Laurent Bigonville wrote:
> Hi,
> 
> With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system
> dbus daemon is complaining with the following message:
> 
> nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC
> avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon"
> sauid=102 hostname=? addr=? terminal=?
> 
> This is the system dbus daemon running as "messagebus":
> 
> message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11
> /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile
> --systemd-activation
> 
> Looking at the capabilities:
> 
> $ sudo getpcaps 1057
> Capabilities for `1057': = cap_audit_write+ep
> 
> All other user_avc seems to be properly logged in audit.
> 
> An idea?

I'd patch it to syslog errno and other information to locate the syscall 
that's failing. Did socket fail? Did the send fail? Does it work in permissive 
mode?

-Steve

^ permalink raw reply

* SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-03 16:05 UTC (permalink / raw)
  To: linux-audit

Hi,

With dbus 1.10.2 (on Debian), when I'm running "semodule -B", the system 
dbus daemon is complaining with the following message:

nov 03 15:02:57 soldur dbus[1057]: Can't send to audit system: USER_AVC 
avc:  received policyload notice (seqno=3) exe="/usr/bin/dbus-daemon" 
sauid=102 hostname=? addr=? terminal=?

This is the system dbus daemon running as "messagebus":

message+  1057  0.0  0.0 127756  4524 ?        Ssl  10:39   0:11 
/usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile 
--systemd-activation

Looking at the capabilities:

$ sudo getpcaps 1057
Capabilities for `1057': = cap_audit_write+ep

All other user_avc seems to be properly logged in audit.

An idea?

Cheers,

Laurent Bigonville

^ permalink raw reply

* Re: audit log still getting rotated even with max_log_file_action = ignore?
From: Steve Grubb @ 2015-11-02 23:32 UTC (permalink / raw)
  To: linux-audit
In-Reply-To: <5637D841.3090501@jlbond.com>

On Monday, November 02, 2015 01:40:17 PM Bond Masuda wrote:
> I'm seeing my /var/log/audit/audit.log getting rotated (I find a audit.1
> or audit.2, etc. file) even though I have max_log_file_action=ignore.
> Here's the full auditd.conf:
> 
> log_file = /var/log/audit/audit.log
> log_format = RAW
> log_group = root
> priority_boost = 4
> flush = INCREMENTAL
> freq = 20
> num_logs = 5
> disp_qos = lossy
> dispatcher = /sbin/audispd
> name_format = hostname
> max_log_file = 6
> max_log_file_action = ignore
> space_left = 75
> space_left_action = email
> action_mail_acct = root
> admin_space_left = 50
> admin_space_left_action = exec /usr/local/bin/remove_oldest_audit_log
> disk_full_action = exec /usr/local/bin/remove_oldest_audit_log
> disk_error_action = SUSPEND
> tcp_listen_queue = 5
> tcp_max_per_addr = 1
> tcp_client_max_idle = 0
> enable_krb5 = no
> krb5_principal = auditd
> 
> what am I missing?

I took a quick look at the code. I can't see how this is happening unless 
auditd is receiving a SIGUSR1 signal.

You might want to put some syslog calls in to auditd-event.c log when auditd 
gets told to rotate so that it can be correlated to other system activities.

-Steve
 

> I have a cron job in /etc/cron.daily/auditd that I use to rotate +
> compress the audit logs, but this is not what is causing the audit log
> rotation.
> 
> Is there another setting I must set in order for it to not automatically
> rotate the audit log? How do I achieve the desired effect, where the
> audit log is only rotated when my cron script runs?

^ permalink raw reply

* audit log still getting rotated even with max_log_file_action = ignore?
From: Bond Masuda @ 2015-11-02 21:40 UTC (permalink / raw)
  To: linux-audit

I'm seeing my /var/log/audit/audit.log getting rotated (I find a audit.1
or audit.2, etc. file) even though I have max_log_file_action=ignore.
Here's the full auditd.conf:

log_file = /var/log/audit/audit.log
log_format = RAW
log_group = root
priority_boost = 4
flush = INCREMENTAL
freq = 20
num_logs = 5
disp_qos = lossy
dispatcher = /sbin/audispd
name_format = hostname
max_log_file = 6
max_log_file_action = ignore
space_left = 75
space_left_action = email
action_mail_acct = root
admin_space_left = 50
admin_space_left_action = exec /usr/local/bin/remove_oldest_audit_log
disk_full_action = exec /usr/local/bin/remove_oldest_audit_log
disk_error_action = SUSPEND
tcp_listen_queue = 5
tcp_max_per_addr = 1
tcp_client_max_idle = 0
enable_krb5 = no
krb5_principal = auditd

what am I missing?

I have a cron job in /etc/cron.daily/auditd that I use to rotate +
compress the audit logs, but this is not what is causing the audit log
rotation.

Is there another setting I must set in order for it to not automatically
rotate the audit log? How do I achieve the desired effect, where the
audit log is only rotated when my cron script runs?

Thanks,
Bond

^ permalink raw reply

* Re: [RFC PATCH v3 2/5] lsm: introduce hooks for kdbus
From: Paul Moore @ 2015-10-29 20:38 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Stephen Smalley, linux-security-module, linux-audit, selinux,
	Paul Osmialowski
In-Reply-To: <CAB9W1A3+oEuKD4QQfogFvrEkRkKF0=y0j86ueQNnePf3P_K+qg@mail.gmail.com>

On Tuesday, October 20, 2015 04:41:14 PM Stephen Smalley wrote:
> On Mon, Oct 19, 2015 at 6:29 PM, Paul Moore <pmoore@redhat.com> wrote:
> > On Friday, October 09, 2015 10:56:12 AM Stephen Smalley wrote:
> >> On 10/07/2015 07:08 PM, Paul Moore wrote:
> >> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> >> > index ef63d65..1cb87b3 100644
> >> > --- a/ipc/kdbus/connection.c
> >> > +++ b/ipc/kdbus/connection.c
> >> > @@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct
> >> > kdbus_ep *ep,>
> >> > 
> >> >     if (!owner && (creds || pids || seclabel))
> >> >     
> >> >             return ERR_PTR(-EPERM);
> >> > 
> >> > +   ret = security_kdbus_conn_new(get_cred(file->f_cred),
> >> 
> >> You only need to use get_cred() if saving a reference; otherwise, you'll
> >> leak one here.
> > 
> > Yes, that was a typo on my part, thanks.
> > 
> >> Also, do we want file->f_cred here or ep->bus->node.creds (the latter is
> >> what is used by their own checks; the former is typically the same as
> >> current cred IIUC).  For that matter, what about ep->node.creds vs
> >> ep->bus-
> >> node.creds vs. ep->bus->domain->node.creds?  Can they differ?  Do we
> >> care?
> > 
> > We don't want file->f_cred, per our previous discussions.  I was working
> > on this patchset in small chunks and while I added credential storing in
> > the nodes, I forgot to update the hooks before I hit send, my apologies.
> > 
> > My current thinking is to pass both the endpoint and bus credentials, as I
> > believe they can differ.  Both the bus and the endpoint inherit their
> > security labels from their creator and while I don't have any specifics,
> > I think it is reasonable to imagine those two processes having different
> > security labels. Assuming we pass both credentials down to the LSM, I'm
> > currently thinking of> 
> > the following SELinux access controls for this hook:
> >   allow <current> bus_t:kdbus { connect };
> >   allow <current> ep_t:kdbus { use privileged activator monitor policy };
> 
> I think it would be simpler to apply an associate check when the
> endpoint is created between the endpoint label and the bus label
> (which will typically be the same), and then only check based on
> endpoint label for all subsequent permission checks involving that
> endpoint.  Then you don't have to worry about which label to use for
> all the other permission checks. And you get finer-grained control -
> per-endpoint rather than only per-bus.

After thinking about this for a bit, I agree.

> > ... besides the additional label, I added the kdbus:use permission and
> > dropped the kdbus:owner permission.  Considering that the endpoint label,
> > ep_t, in the examples above, could be different from the current process,
> > it seemed reasonable to want to control that interaction and I felt the
> > fd:use permission was the closest existing control so I reused the
> > permission name. I decided to drop the "owner" permission as it really
> > wasn't the useful for anything anymore, it simply indicates that the
> > current task is the DAC owner of the endpoint.
> 
> Can you 'use' an endpoint in any way other than to connect via it?
> If not, I'd just call that connect (won't conflict if you get rid of
> the separate bus check above), or distinguish it via separate classes
> or as connectthrough vs connectto.

I don't believe so; my understanding is that the main point of endpoints is to 
define special kdbus DAC policy.
 
> conn->owner is used to determine whether the caller can fake
> credentials, skip kdbus policy checking, create an activator, monitor,
> or policy holder connection, etc.  Our options are:
>
> 1. Apply a SELinux check when it is set to see if the caller is
> allowed to own the bus based on MAC labels and policy, and if not,
> refuse to create the connection (that's what checking the owner
> permission was doing).
> 
> 2. Separately apply MAC checks over each of those abilities (fake
> creds, override policy, create an activator, monitor, or policy
> holder, etc) when there is an attempt to exercise them (not all during
> connection creation), and selectively deny that ability.  More
> invasive, more potential for breakage for applications that don't
> expect failure if they could create the connection in the first place.
> 
> 3. Treat faking of DAC credentials and skipping of kdbus policy
> checking as not of interest to MAC, leaving it only controlled by the
> existing uid match or CAP_IPC_OWNER check.  Simple, but seems to lose
> some of the potential benefit of using SELinux for confining processes
> using kdbus.

I agree with you on #3 so let's rule that out.  The same with #2 as I fear we 
would likely get a nasty surprise at some point when the kdbus folks changed 
something and forgot to update the LSM hooks.  That leaves us with option #1 
and the following operations:

* fake credentials
We are already checking this with the kdbus:impersonate permission.

* skip kdbus policy checks
Being an "owner" allows you to skip the kdbus DAC checks, but the operation 
should still be subject to the LSM hooks.  This really is an issue regardless 
of what we do with respect to checking ownership.

* create an activator
This is still passed as a flag into the security_kdbus_conn_new() hook.

* create a monitor
This is still passed as a flag into the security_kdbus_conn_new() hook.

* create a policy holder
This is still passed as a flag into the security_kdbus_conn_new() hook.

For the sake of completeness we can still keep passing it down via the LSM 
hook, I'm just not sure that adding it to the SELinux policy provides any real 
benefit.

> privileged actually seems less interesting than owner these days,
> although I haven't done a thorough analysis.

The only thing that it appears to offer is the ability to set the connection's 
creating user to a specified value.  From what I'm able to tell, this seems 
more relevant to the DAC side of things than the MAC side, although if we feel 
that "owner" is significant then privileged will remain important as well.

> IIUC, creating a monitor or policy holder connection has bus-wide
> implications and we ought to control it so we can limit it to specific
> processes, not just all uid-0 processes for the system bus and not
> just all uid-<user> processes for the per-user bus.

Agreed.  We still pass those flags to the LSM.

> I'm not actually clear on the implications of activator connections or why
> they are limited in the same way.

My understanding is that they serve as some sort of placeholder for kdbus 
services.  Regardless, we continue to pass the flag through the LSM hook.

> >> > @@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct
> >> > kdbus_conn
> >> > *conn,>
> >> > 
> >> >                     return false;
> >> >     
> >> >     }
> >> > 
> >> > -   if (conn->owner)
> >> > -           return true;
> >> > +   if (!conn->owner &&
> >> > +       kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds, name,
> >> > +                          hash) < KDBUS_POLICY_OWN)
> >> > +           return false;
> >> > 
> >> > -   res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
> >> > -                            name, hash);
> >> > -   return res >= KDBUS_POLICY_OWN;
> >> > +   return (security_kdbus_own_name(conn_creds, name) == 0);
> >> 
> >> Similar question here.  conn_creds is the credentials of the creator of
> >> the connection, typically the client/sender, right?
> >> conn->ep->bus->node.creds are the credentials of the bus owner, so don't
> >> we want to ask "Can I own this name on this bus?".
> > 
> > Yes, I think so.
> > 
> > From a SELinux point of view I imagine we would want access controls along
> > the lines of the following:
> >   allow current name_t:kdbus { own_name };
> >   allow current bus_t:kdbus { own_name };
> > 
> > ... do we want to use different permissions?  I doubt it would matter much
> > either way.
> 
> If we keep them both, then they need separate classes or separate
> permissions; I can't think of another case where we reuse the same
> class and permission for two different objects.
> 
> Not clear that this kind of pairwise checking will work well.  For
> example, I should be able to own any name I like on my own bus, but
> not on the system bus.

If we care both about the service and the bus on which it lives, I think we 
need to somehow factor both into the SELinux bus/service name mapping. 

Perhaps we transition the service names based on the bus; for example, it is 
really on the system bus services that we are likely to care about having 
special/non-default names, isn't it?  Service names on a user bus could simply 
take on the same label as the process.

If we don't think the transitions are the right solution, then I think we'll 
need to add the bus information to the service name/label mapping.  Perhaps 
<bus_label>/<service_name> to <service_label>?  Although my current thinking 
is that the transitions are the way to go.

> >> > @@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct
> >> > kdbus_conn *conn,>
> >> > 
> >> >                                      const struct cred *conn_creds,
> >> >                                      const char *name)
> >> >   
> >> >   {
> >> > 
> >> > -   int res;
> >> > +   if (!conn_creds)
> >> > +           conn_creds = conn->cred;
> >> > 
> >> >     /*
> >> >     
> >> >      * By default, all names are visible on a bus. SEE policies can
> >> >      only be
> >> >      * installed on custom endpoints, where by default no name is
> >> >      visible.
> >> >      */
> >> > 
> >> > -   if (!conn->ep->user)
> >> > -           return true;
> >> > +   if (conn->ep->user &&
> >> > +       kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds,
> >> > name,
> >> > +                                   kdbus_strhash(name)) <
> >> > KDBUS_POLICY_SEE) +           return false;
> >> > 
> >> > -   res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
> >> > -                                     conn_creds ? : conn->cred,
> >> > -                                     name, kdbus_strhash(name));
> >> > -   return res >= KDBUS_POLICY_SEE;
> >> > +   return (security_kdbus_conn_see_name(conn_creds, name) == 0);
> >> 
> >> Here they only define policy based on endpoints, not bus.  Not sure what
> >> we want, but we need at least one of their creds.  Same for the rest.
> > 
> > Once again, I'm not sure we care about the underlying bus at this point,
> > do we?  We've already authorized both the service and the client to
> > connect to the bus, now I think all we care about is can the client see
> > the service name. Do we really care about the label of the bus here?
> 
> Well, here we again have the issue that we should be able to see all
> names on our own bus, but not necessarily on the system bus.

See my comment above, but I think we need to map both the bus and service name 
to a service label.

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: [PATCH] audit: removing unused variable
From: Paul Moore @ 2015-10-29 14:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: Saurabh Sengar, Eric Paris, linux-audit, linux-kernel
In-Reply-To: <1446070373.2757.147.camel@perches.com>

On Wed, Oct 28, 2015 at 6:12 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2015-10-28 at 16:35 -0400, Paul Moore wrote:
>> On Wednesday, October 28, 2015 09:40:34 AM Saurabh Sengar wrote:
>> > variavle rc in not required as it is just used for unchanged for return,
>> > and return is always 0 in the function.
> []
>> Thanks, applied with some spelling corrections to the description.
>
> As the return value is never actually tested,
> it seems better to make it a void function,

Agreed, I did just that yesterday.

 * https://www.redhat.com/archives/linux-audit/2015-October/msg00125.html

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* [PATCH v3] audit: removing unused variable
From: Saurabh Sengar @ 2015-10-29  4:37 UTC (permalink / raw)
  To: paul, eparis, linux-audit, linux-kernel; +Cc: Saurabh Sengar

variable rc is unnecessary hence removing it,
also as the return type of function audit_log_common_recv_msg is no
where used changing it to void.

Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
---
 kernel/audit.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 662c007..a89b2b1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -684,25 +684,22 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 	return err;
 }
 
-static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
+static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
 {
-	int rc = 0;
 	uid_t uid = from_kuid(&init_user_ns, current_uid());
 	pid_t pid = task_tgid_nr(current);
 
 	if (!audit_enabled && msg_type != AUDIT_USER_AVC) {
 		*ab = NULL;
-		return rc;
+		return;
 	}
 
 	*ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
 	if (unlikely(!*ab))
-		return rc;
+		return;
 	audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
 	audit_log_session_info(*ab);
 	audit_log_task_context(*ab);
-
-	return rc;
 }
 
 int is_audit_feature_set(int i)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v2] audit: removing unused variable
From: Joe Perches @ 2015-10-29  4:32 UTC (permalink / raw)
  To: Saurabh Sengar; +Cc: paul, eparis, linux-audit, linux-kernel
In-Reply-To: <1446091912-6001-1-git-send-email-saurabh.truth@gmail.com>

On Thu, 2015-10-29 at 09:41 +0530, Saurabh Sengar wrote:
> variable rc is unnecessary hence removing it,
> also as the return type of function audit_log_common_recv_msg is no
> where used changing it to void.

Almost, but not quite.  Keep at it.

Ideally your first attempts at kernel patching
should be done in the drivers/staging/ directory
before attempting patches outside of that path.

> diff --git a/kernel/audit.c b/kernel/audit.c
[]
> @@ -684,25 +684,24 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
>  	return err;
>  }
>  
> -static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> +static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
>  {
> -	int rc = 0;
>  	uid_t uid = from_kuid(&init_user_ns, current_uid());
>  	pid_t pid = task_tgid_nr(current);
>  
>  	if (!audit_enabled && msg_type != AUDIT_USER_AVC) {
>  		*ab = NULL;
> -		return rc;
> +		return ;

There's an unnecessary space before the semicolon.
This should be
		return;

>  	}
>  
>  	*ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
>  	if (unlikely(!*ab))
> -		return rc;
> +		return ;

here too

>  	audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
>  	audit_log_session_info(*ab);
>  	audit_log_task_context(*ab);
>  
> -	return rc;
> +	return ;

No return statement necessary here at all

>  }
>  
>  int is_audit_feature_set(int i)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox