Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1532512 > unrolled thread

Re: netlink: GPF in sock_sndtimeo

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-11-29 18:00 +0100
    Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-11-30 00:20 +0100
      Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-11-30 06:00 +0100
        Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-09 07:10 +0100
          Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-09 08:00 +0100
            Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-09 12:10 +0100
              Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-10 05:20 +0100
                Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-10 08:50 +0100
          Re: netlink: GPF in sock_sndtimeo Dmitry Vyukov <dvyukov@google.com> - 2016-12-09 11:50 +0100
            Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-09 12:50 +0100
              Re: netlink: GPF in sock_sndtimeo Dmitry Vyukov <dvyukov@google.com> - 2016-12-09 13:00 +0100
                Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-09 13:20 +0100

#1532512 — Re: netlink: GPF in sock_sndtimeo

On 2016-11-26 17:11, Cong Wang wrote:
> On Sat, Nov 26, 2016 at 7:44 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > Hello,

Eric, thanks for the heads up on this.  (more below...)

> > The following program triggers GPF in sock_sndtimeo:
> > https://gist.githubusercontent.com/dvyukov/c19cadd309791cf5cb9b2bf936d3f48d/raw/1743ba0211079a5465d039512b427bc6b59b1a76/gistfile1.txt
> >
> > On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).
> >
> > general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
> > Dumping ftrace buffer:
> >    (ftrace buffer empty)
> > Modules linked in:
> > CPU: 1 PID: 19950 Comm: syz-executor Not tainted 4.9.0-rc5+ #54
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > task: ffff88002a0d0840 task.stack: ffff880036920000
> > RIP: 0010:[<ffffffff86cb35e1>]  [<     inline     >] sock_sndtimeo
> > include/net/sock.h:2075
> > RIP: 0010:[<ffffffff86cb35e1>]  [<ffffffff86cb35e1>]
> > netlink_unicast+0xe1/0x730 net/netlink/af_netlink.c:1232
> > RSP: 0018:ffff880036926f68  EFLAGS: 00010202
> > RAX: 0000000000000068 RBX: ffff880036927000 RCX: ffffc900021d0000
> > RDX: 0000000000000d63 RSI: 00000000024000c0 RDI: 0000000000000340
> > RBP: ffff880036927028 R08: ffffed0006ea7aab R09: ffffed0006ea7aab
> > R10: 0000000000000001 R11: ffffed0006ea7aaa R12: dffffc0000000000
> > R13: 0000000000000000 R14: ffff880035de3400 R15: ffff880035de3400
> > FS:  00007f90a2fc7700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00000000006de0c0 CR3: 0000000035de6000 CR4: 00000000000006e0
> > Stack:
> >  ffff880035de3400 ffffffff819f02a1 1ffff10006d24df4 0000000000000004
> >  00004db400000014 ffff880036926fd8 ffffffff00000000 0000000041b58ab3
> >  ffffffff89653c11 ffffffff86cb3500 ffffffff819f0345 ffff880035de3400
> > Call Trace:
> >  [<     inline     >] audit_replace kernel/audit.c:817
> >  [<ffffffff816c34b9>] audit_receive_msg+0x22c9/0x2ce0 kernel/audit.c:894
> >  [<     inline     >] audit_receive_skb kernel/audit.c:1120
> >  [<ffffffff816c40ac>] audit_receive+0x1dc/0x360 kernel/audit.c:1133
> >  [<     inline     >] netlink_unicast_kernel net/netlink/af_netlink.c:1214
> >  [<ffffffff86cb3a14>] netlink_unicast+0x514/0x730 net/netlink/af_netlink.c:1240
> >  [<ffffffff86cb46d4>] netlink_sendmsg+0xaa4/0xe50 net/netlink/af_netlink.c:1786
> >  [<     inline     >] sock_sendmsg_nosec net/socket.c:621
> >  [<ffffffff86a6d54f>] sock_sendmsg+0xcf/0x110 net/socket.c:631
> >  [<ffffffff86a6d8bb>] sock_write_iter+0x32b/0x620 net/socket.c:829
> >  [<     inline     >] new_sync_write fs/read_write.c:499
> >  [<ffffffff81a6f24e>] __vfs_write+0x4fe/0x830 fs/read_write.c:512
> >  [<ffffffff81a70cf5>] vfs_write+0x175/0x4e0 fs/read_write.c:560
> >  [<     inline     >] SYSC_write fs/read_write.c:607
> >  [<ffffffff81a75180>] SyS_write+0x100/0x240 fs/read_write.c:599
> >  [<ffffffff81009a24>] do_syscall_64+0x2f4/0x940 arch/x86/entry/common.c:280
> >  [<ffffffff88149e8d>] entry_SYSCALL64_slow_path+0x25/0x25
> > Code: fe 4c 89 f7 e8 31 16 ff ff 8b 8d 70 ff ff ff 49 89 c7 31 c0 85
> > c9 75 25 e8 7d 4a a3 fa 49 8d bd 40 03 00 00 48 89 f8 48 c1 e8 03 <42>
> > 80 3c 20 00 0f 85 3a 06 00 00 49 8b 85 40 03 00 00 4c 8d 73
> > RIP  [<     inline     >] sock_sndtimeo include/net/sock.h:2075
> > RIP  [<ffffffff86cb35e1>] netlink_unicast+0xe1/0x730
> > net/netlink/af_netlink.c:1232
> >  RSP <ffff880036926f68>
> > ---[ end trace 8383a15fba6fdc59 ]---
> 
> It is racy on audit_sock, especially on the netns exit path.

I think that is the only place it is racy.  The other places audit_sock
is set is when the socket failure has just triggered a reset.

Is there a notifier callback for failed or reaped sockets?

> Could the following patch help a little bit? Also, I don't see how the
> synchronize_net() here could sync with netlink rcv path, since unlike
> packets from wire, netlink messages are not handled in BH context
> nor I see any RCU taken on rcv path.

Thanks Cong, this looks like it could help.  I'll have a closer look.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index f1ca116..20bc79e 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1167,10 +1167,13 @@ static void __net_exit audit_net_exit(struct net *net)
>  {
>         struct audit_net *aunet = net_generic(net, audit_net_id);
>         struct sock *sock = aunet->nlsk;
> +
> +       mutex_lock(&audit_cmd_mutex);
>         if (sock == audit_sock) {
>                 audit_pid = 0;
>                 audit_sock = NULL;
>         }
> +       mutex_unlock(&audit_cmd_mutex);
> 
>         RCU_INIT_POINTER(aunet->nlsk, NULL);
>         synchronize_net();

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [next] | [standalone]

#1532840

On Tue, Nov 29, 2016 at 8:48 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2016-11-26 17:11, Cong Wang wrote:
>> It is racy on audit_sock, especially on the netns exit path.
>
> I think that is the only place it is racy.  The other places audit_sock
> is set is when the socket failure has just triggered a reset.
>
> Is there a notifier callback for failed or reaped sockets?

Is NETLINK_URELEASE event what you are looking for?

[toc] | [prev] | [next] | [standalone]

#1532953

On 2016-11-29 15:13, Cong Wang wrote:
> On Tue, Nov 29, 2016 at 8:48 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2016-11-26 17:11, Cong Wang wrote:
> >> It is racy on audit_sock, especially on the netns exit path.
> >
> > I think that is the only place it is racy.  The other places audit_sock
> > is set is when the socket failure has just triggered a reset.
> >
> > Is there a notifier callback for failed or reaped sockets?
> 
> Is NETLINK_URELEASE event what you are looking for?

Possibly, yes.  Thanks, I'll have a look.

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539119

On 2016-11-29 23:52, Richard Guy Briggs wrote:
> On 2016-11-29 15:13, Cong Wang wrote:
> > On Tue, Nov 29, 2016 at 8:48 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2016-11-26 17:11, Cong Wang wrote:
> > >> It is racy on audit_sock, especially on the netns exit path.
> > >
> > > I think that is the only place it is racy.  The other places audit_sock
> > > is set is when the socket failure has just triggered a reset.
> > >
> > > Is there a notifier callback for failed or reaped sockets?
> > 
> > Is NETLINK_URELEASE event what you are looking for?
> 
> Possibly, yes.  Thanks, I'll have a look.

I tried a quick compile attempt on the test case (I assume it is a
socket fuzzer) and get the following compile error:
cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
<command-line>: warning: this is the location of the previous definition
socket_fuzz.c: In function ‘segv_handler’:
socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
socket_fuzz.c:89: error: for each function it appears in.)
socket_fuzz.c: In function ‘loop’:
socket_fuzz.c:280: warning: unused variable ‘errno0’
socket_fuzz.c: In function ‘test’:
socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’

I also tried to extend Cong Wang's idea to attempt to proactively respond to a
NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
Eliminating the lock since the sock is dead anways eliminates the error.

Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
get the test case to compile.

This is being tracked as https://github.com/linux-audit/audit-kernel/issues/30

Subject: [PATCH] audit: proactively reset audit_sock on matching NETLINK_URELEASE

diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..91d222d 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -423,6 +423,7 @@ static void kauditd_send_skb(struct sk_buff *skb)
 				snprintf(s, sizeof(s), "audit_pid=%d reset", audit_pid);
 				audit_log_lost(s);
 				audit_pid = 0;
+				audit_nlk_portid = 0;
 				audit_sock = NULL;
 			} else {
 				pr_warn("re-scheduling(#%d) write to audit_pid=%d\n",
@@ -1143,6 +1144,28 @@ static int audit_bind(struct net *net, int group)
 	return 0;
 }
 
+static int audit_sock_netlink_notify(struct notifier_block *nb,
+				     unsigned long event,
+				     void *_notify)
+{
+	struct netlink_notify *notify = _notify;
+	struct audit_net *aunet = net_generic(notify->net, audit_net_id);
+
+	if (event == NETLINK_URELEASE && notify->protocol == NETLINK_AUDIT) {
+		if (audit_nlk_portid == notify->portid &&
+		    audit_sock == aunet->nlsk) {
+			audit_pid = 0;
+			audit_nlk_portid = 0;
+			audit_sock = NULL;
+		}
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block audit_netlink_notifier = {
+	.notifier_call = audit_sock_netlink_notify,
+};
+
 static int __net_init audit_net_init(struct net *net)
 {
 	struct netlink_kernel_cfg cfg = {
@@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
 {
 	struct audit_net *aunet = net_generic(net, audit_net_id);
 	struct sock *sock = aunet->nlsk;
+
+	mutex_lock(&audit_cmd_mutex);
 	if (sock == audit_sock) {
 		audit_pid = 0;
+		audit_nlk_portid = 0;
 		audit_sock = NULL;
 	}
+	mutex_unlock(&audit_cmd_mutex);
 
 	RCU_INIT_POINTER(aunet->nlsk, NULL);
 	synchronize_net();
@@ -1202,6 +1229,7 @@ static int __init audit_init(void)
 	audit_enabled = audit_default;
 	audit_ever_enabled |= !!audit_default;
 
+	netlink_register_notifier(&audit_netlink_notifier);
 	audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
 
 	for (i = 0; i < AUDIT_INODE_BUCKETS; i++)
-- 
1.7.1


> - RGB

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539140

On Thu, Dec 8, 2016 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> I also tried to extend Cong Wang's idea to attempt to proactively respond to a
> NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
> stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
> Eliminating the lock since the sock is dead anways eliminates the error.
>
> Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
> get the test case to compile.

It doesn't look safe, because 'audit_sock', 'audit_nlk_portid' and 'audit_pid'
are updated as a whole and race between audit_receive_msg() and
NETLINK_URELEASE.


> @@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
>  {
>         struct audit_net *aunet = net_generic(net, audit_net_id);
>         struct sock *sock = aunet->nlsk;
> +
> +       mutex_lock(&audit_cmd_mutex);
>         if (sock == audit_sock) {
>                 audit_pid = 0;
> +               audit_nlk_portid = 0;
>                 audit_sock = NULL;
>         }
> +       mutex_unlock(&audit_cmd_mutex);
>

If you decide to use NETLINK_URELEASE notifier, the above piece is no
longer needed, the net_exit path simply releases a refcnt.

[toc] | [prev] | [next] | [standalone]

#1539278

On 2016-12-08 22:57, Cong Wang wrote:
> On Thu, Dec 8, 2016 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > I also tried to extend Cong Wang's idea to attempt to proactively respond to a
> > NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
> > stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
> > Eliminating the lock since the sock is dead anways eliminates the error.
> >
> > Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
> > get the test case to compile.
> 
> It doesn't look safe, because 'audit_sock', 'audit_nlk_portid' and 'audit_pid'
> are updated as a whole and race between audit_receive_msg() and
> NETLINK_URELEASE.

This is what I expected and why I originally added the mutex lock in the
callback...  The dumps I got were bare with no wrapper identifying the
process context or specific error, so I'm at a bit of a loss how to
solve this (without thinking more about it) other than instinctively
removing the mutex.

Another approach might be to look at consolidating the three into one
identifier or derive the other two from one, or serialize their access.

> > @@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
> >  {
> >         struct audit_net *aunet = net_generic(net, audit_net_id);
> >         struct sock *sock = aunet->nlsk;
> > +
> > +       mutex_lock(&audit_cmd_mutex);
> >         if (sock == audit_sock) {
> >                 audit_pid = 0;
> > +               audit_nlk_portid = 0;
> >                 audit_sock = NULL;
> >         }
> > +       mutex_unlock(&audit_cmd_mutex);
> 
> If you decide to use NETLINK_URELEASE notifier, the above piece is no
> longer needed, the net_exit path simply releases a refcnt.

Good point.  It would have already killed it off.  So this piece is
arguably too late anyways.

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539767

On Fri, Dec 9, 2016 at 3:01 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2016-12-08 22:57, Cong Wang wrote:
>> On Thu, Dec 8, 2016 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > I also tried to extend Cong Wang's idea to attempt to proactively respond to a
>> > NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
>> > stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
>> > Eliminating the lock since the sock is dead anways eliminates the error.
>> >
>> > Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
>> > get the test case to compile.
>>
>> It doesn't look safe, because 'audit_sock', 'audit_nlk_portid' and 'audit_pid'
>> are updated as a whole and race between audit_receive_msg() and
>> NETLINK_URELEASE.
>
> This is what I expected and why I originally added the mutex lock in the
> callback...  The dumps I got were bare with no wrapper identifying the
> process context or specific error, so I'm at a bit of a loss how to
> solve this (without thinking more about it) other than instinctively
> removing the mutex.

Netlink notifier can safely be converted to blocking one, I will send
a patch.

But I seriously doubt you really need NETLINK_URELEASE here,
it adds nothing but overhead, b/c the netlink notifier is called on
every netlink socket in the system, but for net exit path, that is
relatively a slow path.

Also, kauditd_send_skb() needs audit_cmd_mutex too.

I will send a formal patch.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1539793

[Multipart message — attachments visible in raw view] — view raw

On Fri, Dec 9, 2016 at 8:13 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Dec 9, 2016 at 3:01 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> On 2016-12-08 22:57, Cong Wang wrote:
>>> On Thu, Dec 8, 2016 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>>> > I also tried to extend Cong Wang's idea to attempt to proactively respond to a
>>> > NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
>>> > stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
>>> > Eliminating the lock since the sock is dead anways eliminates the error.
>>> >
>>> > Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
>>> > get the test case to compile.
>>>
>>> It doesn't look safe, because 'audit_sock', 'audit_nlk_portid' and 'audit_pid'
>>> are updated as a whole and race between audit_receive_msg() and
>>> NETLINK_URELEASE.
>>
>> This is what I expected and why I originally added the mutex lock in the
>> callback...  The dumps I got were bare with no wrapper identifying the
>> process context or specific error, so I'm at a bit of a loss how to
>> solve this (without thinking more about it) other than instinctively
>> removing the mutex.
>
> Netlink notifier can safely be converted to blocking one, I will send
> a patch.
>
> But I seriously doubt you really need NETLINK_URELEASE here,
> it adds nothing but overhead, b/c the netlink notifier is called on
> every netlink socket in the system, but for net exit path, that is
> relatively a slow path.
>
> Also, kauditd_send_skb() needs audit_cmd_mutex too.

Please let me know what you think about the attached patch?

Thanks!

[toc] | [prev] | [next] | [standalone]

#1539251

On Fri, Dec 9, 2016 at 7:02 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2016-11-29 23:52, Richard Guy Briggs wrote:
>> On 2016-11-29 15:13, Cong Wang wrote:
>> > On Tue, Nov 29, 2016 at 8:48 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > > On 2016-11-26 17:11, Cong Wang wrote:
>> > >> It is racy on audit_sock, especially on the netns exit path.
>> > >
>> > > I think that is the only place it is racy.  The other places audit_sock
>> > > is set is when the socket failure has just triggered a reset.
>> > >
>> > > Is there a notifier callback for failed or reaped sockets?
>> >
>> > Is NETLINK_URELEASE event what you are looking for?
>>
>> Possibly, yes.  Thanks, I'll have a look.
>
> I tried a quick compile attempt on the test case (I assume it is a
> socket fuzzer) and get the following compile error:
> cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
> socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
> <command-line>: warning: this is the location of the previous definition
> socket_fuzz.c: In function ‘segv_handler’:
> socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
> socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
> socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
> socket_fuzz.c:89: error: for each function it appears in.)
> socket_fuzz.c: In function ‘loop’:
> socket_fuzz.c:280: warning: unused variable ‘errno0’
> socket_fuzz.c: In function ‘test’:
> socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
> socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
> socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’

-std=gnu99 should help
ignore warnings



> I also tried to extend Cong Wang's idea to attempt to proactively respond to a
> NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
> stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
> Eliminating the lock since the sock is dead anways eliminates the error.
>
> Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
> get the test case to compile.
>
> This is being tracked as https://github.com/linux-audit/audit-kernel/issues/30
>
> Subject: [PATCH] audit: proactively reset audit_sock on matching NETLINK_URELEASE
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f1ca116..91d222d 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -423,6 +423,7 @@ static void kauditd_send_skb(struct sk_buff *skb)
>                                 snprintf(s, sizeof(s), "audit_pid=%d reset", audit_pid);
>                                 audit_log_lost(s);
>                                 audit_pid = 0;
> +                               audit_nlk_portid = 0;
>                                 audit_sock = NULL;
>                         } else {
>                                 pr_warn("re-scheduling(#%d) write to audit_pid=%d\n",
> @@ -1143,6 +1144,28 @@ static int audit_bind(struct net *net, int group)
>         return 0;
>  }
>
> +static int audit_sock_netlink_notify(struct notifier_block *nb,
> +                                    unsigned long event,
> +                                    void *_notify)
> +{
> +       struct netlink_notify *notify = _notify;
> +       struct audit_net *aunet = net_generic(notify->net, audit_net_id);
> +
> +       if (event == NETLINK_URELEASE && notify->protocol == NETLINK_AUDIT) {
> +               if (audit_nlk_portid == notify->portid &&
> +                   audit_sock == aunet->nlsk) {
> +                       audit_pid = 0;
> +                       audit_nlk_portid = 0;
> +                       audit_sock = NULL;
> +               }
> +       }
> +       return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block audit_netlink_notifier = {
> +       .notifier_call = audit_sock_netlink_notify,
> +};
> +
>  static int __net_init audit_net_init(struct net *net)
>  {
>         struct netlink_kernel_cfg cfg = {
> @@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
>  {
>         struct audit_net *aunet = net_generic(net, audit_net_id);
>         struct sock *sock = aunet->nlsk;
> +
> +       mutex_lock(&audit_cmd_mutex);
>         if (sock == audit_sock) {
>                 audit_pid = 0;
> +               audit_nlk_portid = 0;
>                 audit_sock = NULL;
>         }
> +       mutex_unlock(&audit_cmd_mutex);
>
>         RCU_INIT_POINTER(aunet->nlsk, NULL);
>         synchronize_net();
> @@ -1202,6 +1229,7 @@ static int __init audit_init(void)
>         audit_enabled = audit_default;
>         audit_ever_enabled |= !!audit_default;
>
> +       netlink_register_notifier(&audit_netlink_notifier);
>         audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
>
>         for (i = 0; i < AUDIT_INODE_BUCKETS; i++)
> --
> 1.7.1
>
>
>> - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539299

On 2016-12-09 11:49, Dmitry Vyukov wrote:
> On Fri, Dec 9, 2016 at 7:02 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2016-11-29 23:52, Richard Guy Briggs wrote:
> > I tried a quick compile attempt on the test case (I assume it is a
> > socket fuzzer) and get the following compile error:
> > cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
> > socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
> > <command-line>: warning: this is the location of the previous definition
> > socket_fuzz.c: In function ‘segv_handler’:
> > socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
> > socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
> > socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
> > socket_fuzz.c:89: error: for each function it appears in.)
> > socket_fuzz.c: In function ‘loop’:
> > socket_fuzz.c:280: warning: unused variable ‘errno0’
> > socket_fuzz.c: In function ‘test’:
> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
> > socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’
> 
> -std=gnu99 should help
> ignore warnings

I got a little further, left with "__ATOMIC_RELAXED undeclared", "__ATOMIC_SEQ_CST
undeclared" under gcc 4.4.7-16.

gcc 4.8.2-15 leaves me with "undefined reference to `clock_gettime'"

What compiler version do you recommend?

> >> - RGB
> >
> > - RGB

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539304

On Fri, Dec 9, 2016 at 12:48 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2016-12-09 11:49, Dmitry Vyukov wrote:
>> On Fri, Dec 9, 2016 at 7:02 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2016-11-29 23:52, Richard Guy Briggs wrote:
>> > I tried a quick compile attempt on the test case (I assume it is a
>> > socket fuzzer) and get the following compile error:
>> > cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
>> > socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
>> > <command-line>: warning: this is the location of the previous definition
>> > socket_fuzz.c: In function ‘segv_handler’:
>> > socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
>> > socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
>> > socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
>> > socket_fuzz.c:89: error: for each function it appears in.)
>> > socket_fuzz.c: In function ‘loop’:
>> > socket_fuzz.c:280: warning: unused variable ‘errno0’
>> > socket_fuzz.c: In function ‘test’:
>> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
>> > socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
>> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’
>>
>> -std=gnu99 should help
>> ignore warnings
>
> I got a little further, left with "__ATOMIC_RELAXED undeclared", "__ATOMIC_SEQ_CST
> undeclared" under gcc 4.4.7-16.
>
> gcc 4.8.2-15 leaves me with "undefined reference to `clock_gettime'"

add -lrt


> What compiler version do you recommend?

6.x sounds reasonable
4.4 branch is 7.5 years old, surprised that it does not disintegrate
into dust yet :)


>> >> - RGB
>> >
>> > - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1539313

On 2016-12-09 12:53, Dmitry Vyukov wrote:
> On Fri, Dec 9, 2016 at 12:48 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2016-12-09 11:49, Dmitry Vyukov wrote:
> >> On Fri, Dec 9, 2016 at 7:02 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > On 2016-11-29 23:52, Richard Guy Briggs wrote:
> >> > I tried a quick compile attempt on the test case (I assume it is a
> >> > socket fuzzer) and get the following compile error:
> >> > cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
> >> > socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
> >> > <command-line>: warning: this is the location of the previous definition
> >> > socket_fuzz.c: In function ‘segv_handler’:
> >> > socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
> >> > socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
> >> > socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
> >> > socket_fuzz.c:89: error: for each function it appears in.)
> >> > socket_fuzz.c: In function ‘loop’:
> >> > socket_fuzz.c:280: warning: unused variable ‘errno0’
> >> > socket_fuzz.c: In function ‘test’:
> >> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
> >> > socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
> >> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’
> >>
> >> -std=gnu99 should help
> >> ignore warnings
> >
> > I got a little further, left with "__ATOMIC_RELAXED undeclared", "__ATOMIC_SEQ_CST
> > undeclared" under gcc 4.4.7-16.
> >
> > gcc 4.8.2-15 leaves me with "undefined reference to `clock_gettime'"
> 
> add -lrt

Ok, that helped.  Thanks!

> > What compiler version do you recommend?
> 
> 6.x sounds reasonable
> 4.4 branch is 7.5 years old, surprised that it does not disintegrate
> into dust yet :)

  These are under RHEL6...  so there are updates to them, but yeah, they are old.

> >> >> - RGB
> >> >
> >> > - RGB
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web