Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Paul Moore]

On Monday, December 29, 2014 05:24:38 PM Toralf Förster wrote:
> On 12/29/2014 05:21 PM, Paul Moore wrote:
> > On Mon, Dec 29, 2014 at 11:07 AM, Toralf Förster wrote:
> >> A x86 KVM guest running at a 64 bit Gentoo hardened host system the
> >> following crashes appeared reproducible (screen shots attached.
> >> 
> >> If I removed syslog-ng from the runlevel default, then the crash just
> >> appeared a little bit later at another subsystem> 
> >
> > It looks like it doesn't like something in audit_compare_dname_path();
> > I'll take a look and see what I can find, there is a patch in -rc2
> > which touched some related code.
> > 
> > I didn't see this problem in my earlier testing, can you share your
> > .config?
>
> ofc - attached

[NOTE: added linux-audit to the CC line, I should have done that earlier]

I believe I can reproduce this now; I'm seeing slightly different panics, but 
it is "close enough" and based on some quality time with the code I believe 
they are both symptoms of the same root cause.

To help verify that I'm heading down the right path, could you share your 
audit configuration as well?  If that's not possible, can you at least confirm 
that you using a few audit directory watches?

-- 
paul moore
www.paul-moore.com

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Toralf Förster]

On 12/29/2014 08:41 PM, Paul Moore wrote:
> To help verify that I'm heading down the right path, could you share your 
> audit configuration as well?  If that's not possible, can you at least confirm 
> that you using a few audit directory watches?

Well, it is just a victim system for trinity - but I did not configured auditd in a special manner - so it is just the plain default configuration of Gentoo:

n22kvm-clone audit # wc *
  26  201 1127 audit.rules
  13   85  573 audit.rules.stop.post
  16   81  547 audit.rules.stop.pre
  32   95  701 auditd.conf
  87  462 2948 total
n22kvm-clone audit # tail -n 40 -v *
==> audit.rules <==                                                                                                
# Copyright 1999-2011 Gentoo Foundation                                                                            
# Distributed under the terms of the GNU General Public License v2                                                 
# $Header: /var/cvsroot/gentoo-x86/sys-process/audit/files/audit.rules-2.1.3,v 1.1 2011/09/11 02:58:55 robbat2 Exp $
#                                                                                                                  
# This file contains the auditctl rules that are loaded                                                            
# whenever the audit daemon is started via the initscripts.                                                        
# The rules are simply the parameters that would be passed                                                         
# to auditctl.                                                                                                     
                                                                                                                                  
# First rule - delete all                                                                                                         
# This is to clear out old rules, so we don't append to them.                                                                     
-D                                                                                                                                
                                                                                                                                                    
# Feel free to add below this line. See auditctl man page                                                                                           
                                                                                                                                                    
# The following rule would cause all of the syscalls listed to be ignored in logging.                                                               
-a exit,never -F arch=b32 -S read -S write -S open -S fstat -S mmap -S brk -S munmap -S nanosleep -S fcntl -S close -S dup2 -S rt_sigaction -S stat                          
#-a exit,never -F arch=b64 -S read -S write -S open -S fstat -S mmap -S brk -S munmap -S nanosleep -S fcntl -S close -S dup2 -S rt_sigaction -S stat                         
                                                                                                                                                                             
# The following rule would cause the capture of all systems not caught above.                                                                                                
# -a exit,always -S all                                                                                                                                                      
                                                                                                                                                                             
# Increase the buffers to survive stress events                                                                                                                              
-b 8192                                                                                                                                                                      
                                                                                                                                                                             
# vim:ft=conf:                                                                                                                                                               
                                                                                                                                                                             
==> audit.rules.stop.post <==                                                                                                                                                
# Copyright 1999-2005 Gentoo Foundation                                                                                                                                      
# Distributed under the terms of the GNU General Public License v2                                                                                                           
# $Header: /var/cvsroot/gentoo-x86/sys-process/audit/files/audit.rules.stop.post,v 1.1 2006/06/22 07:41:46 robbat2 Exp $                                                     
#                                                                                                                                                                            
# This file contains the auditctl rules that are loaded immediately after the                                                                                                
# audit deamon is stopped via the initscripts.                                                                                                                               
# The rules are simply the parameters that would be passed
# to auditctl.

# Not used for the default Gentoo configuration as of v1.2.3
# Paranoid security types might wish to reconfigure kauditd here.

# vim:ft=conf:

==> audit.rules.stop.pre <==
# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: /var/cvsroot/gentoo-x86/sys-process/audit/files/audit.rules.stop.pre,v 1.2 2011/09/11 02:58:55 robbat2 Exp $
#
# This file contains the auditctl rules that are loaded immediately before the
# audit deamon is stopped via the initscripts.
# The rules are simply the parameters that would be passed
# to auditctl.

# auditd is stopping, don't capture events anymore
-D

# Disable kernel generating audit events
-e 0

# vim:ft=conf:

==> auditd.conf <==
#
# This file controls the configuration of the audit daemon
#

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 = NONE
##name = mydomain
max_log_file = 6 
max_log_file_action = ROTATE
space_left = 75
space_left_action = SYSLOG
action_mail_acct = root
admin_space_left = 50
admin_space_left_action = SUSPEND
disk_full_action = SUSPEND
disk_error_action = SUSPEND
##tcp_listen_port = 
tcp_listen_queue = 5
tcp_max_per_addr = 1
##tcp_client_ports = 1024-65535
tcp_client_max_idle = 0
enable_krb5 = no
krb5_principal = auditd
##krb5_key_file = /etc/audit/audit.key



n22kvm-clone audit # cat /etc/conf.d/auditd 
# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: /var/cvsroot/gentoo-x86/sys-process/audit/files/auditd-conf.d-2.1.3,v 1.1 2011/09/11 02:58:55 robbat2 Exp $

# Configuration options for auditd
# -f for foreground mode
# There are some other options as well, but you'll have to look in the source
# code to find them as they aren't ready for use yet.
EXTRAOPTIONS=''

# Audit rules file to run after starting auditd
RULEFILE_STARTUP=/etc/audit/audit.rules

# Audit rules file to run before and after stopping auditd
RULEFILE_STOP_PRE=/etc/audit/audit.rules.stop.pre
RULEFILE_STOP_POST=/etc/audit/audit.rules.stop.post

# If you want to enforce a certain locale for auditd, 
# uncomment one of the next lines:
#AUDITD_LANG=none
AUDITD_LANG=C
#AUDITD_LANG=en_US
#AUDITD_LANG=en_US.UTF-8



-- 
Toralf
pgp key: 7B1A 07F4 EC82 0F90 D4C2  8936 872A E508 0076 E94E

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Paul Moore]

On Monday, December 29, 2014 09:18:44 PM Toralf Förster wrote:
> On 12/29/2014 08:41 PM, Paul Moore wrote:
> > To help verify that I'm heading down the right path, could you share your
> > audit configuration as well?  If that's not possible, can you at least
> > confirm that you using a few audit directory watches?
> 
> Well, it is just a victim system for trinity - but I did not configured
> auditd in a special manner - so it is just the plain default configuration
> of Gentoo:

Okay, thanks for the information; the file related syscall watches are likely 
what triggered the problem code.  Until I've got the fix sorted out, removing 
the syscall watches or just disabling auditd from starting at boot should 
workaround the problem.

-- 
paul moore
www.paul-moore.com

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Paul Moore]

On Tuesday, December 30, 2014 09:11:32 AM Paul Moore wrote:
> On Monday, December 29, 2014 09:18:44 PM Toralf Förster wrote:
> > On 12/29/2014 08:41 PM, Paul Moore wrote:
> > > To help verify that I'm heading down the right path, could you share
> > > your audit configuration as well?  If that's not possible, can you at
> > > least confirm that you using a few audit directory watches?
> > 
> > Well, it is just a victim system for trinity - but I did not configured
> > auditd in a special manner - so it is just the plain default configuration
> > of Gentoo:
>
> Okay, thanks for the information; the file related syscall watches are
> likely what triggered the problem code.  Until I've got the fix sorted out,
> removing the syscall watches or just disabling auditd from starting at boot
> should workaround the problem.

I still want to go over the below patch a bit more to check a few things, but 
it solves the problem for me and I believe it should solve the problem you are 
seeing as well.  Can you give it a try and let me know what happens?

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 287b3d3..d834770 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -72,6 +72,8 @@
 #include <linux/fs_struct.h>
 #include <linux/compat.h>
 #include <linux/ctype.h>
+#include <linux/string.h>
+#include <uapi/linux/limits.h>
 
 #include "audit.h"
 
@@ -1862,7 +1864,7 @@ void __audit_inode(struct filename *name, const struct 
dentry *dentry,
 
 	list_for_each_entry_reverse(n, &context->names_list, list) {
 		/* does the name pointer match? */
-		if (!n->name || n->name->name != name->name)
+		if (!n->name || strcmp(n->name->name, name->name))
 			continue;
 
 		/* match the correct record type */
@@ -1881,14 +1883,39 @@ out_alloc:
 	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
 	if (!n)
 		return;
-	if (name)
-		/* since name is not NULL we know there is already a matching
-		 * name record, see audit_getname(), so there must be a type
-		 * mismatch; reuse the string path since the original name
-		 * record will keep the string valid until we free it in
-		 * audit_free_names() */
-		n->name = name;
+	/* unfortunately, while we may have a path name to record with the
+	 * inode, we can't always rely on the string lasting until the end of
+	 * the syscall so we need to create our own copy, it may fail due to
+	 * memory allocation issues, but we do our best */
+	if (name) {
+		/* we can't use getname_kernel() due to size limits */
+		struct filename *new = __getname();
 
+		if (unlikely(!new))
+			goto out;
+
+		memset(new, 0, sizeof(*new));
+		if ((strlen(name->name) + 1) <= (PATH_MAX - sizeof(*new))) {
+			char *new_name = (char *)(new) + sizeof(*new);
+			new->name = new_name;
+			new->separate = false;
+		} else {
+			/* this looks odd, but is due to final_putname() */
+			struct filename *new2;
+			new2 = kzalloc(sizeof(*new2), GFP_KERNEL);
+			if (unlikely(!new2)) {
+				__putname(new);
+				goto out;
+			}
+			new2->name = (char *)new;
+			new = new2;
+			new->separate = true;
+		}
+		strcpy((char *)new->name, name->name);
+		new->aname = n;
+		n->name = new;
+		n->name_put = true;
+	}
 out:
 	if (parent) {
 		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;

-- 
paul moore
www.paul-moore.com

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Toralf Förster]

On 12/30/2014 07:46 PM, Paul Moore wrote:
> On Tuesday, December 30, 2014 09:11:32 AM Paul Moore wrote:
>> On Monday, December 29, 2014 09:18:44 PM Toralf Förster wrote:
>>> On 12/29/2014 08:41 PM, Paul Moore wrote:
>>>> To help verify that I'm heading down the right path, could you share
>>>> your audit configuration as well?  If that's not possible, can you at
>>>> least confirm that you using a few audit directory watches?
>>>
>>> Well, it is just a victim system for trinity - but I did not configured
>>> auditd in a special manner - so it is just the plain default configuration
>>> of Gentoo:
>>
>> Okay, thanks for the information; the file related syscall watches are
>> likely what triggered the problem code.  Until I've got the fix sorted out,
>> removing the syscall watches or just disabling auditd from starting at boot
>> should workaround the problem.
> 
> I still want to go over the below patch a bit more to check a few things, but 
> it solves the problem for me and I believe it should solve the problem you are 
> seeing as well.  Can you give it a try and let me know what happens?
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 287b3d3..d834770 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -72,6 +72,8 @@
>  #include <linux/fs_struct.h>
>  #include <linux/compat.h>
>  #include <linux/ctype.h>
> +#include <linux/string.h>
> +#include <uapi/linux/limits.h>
>  
>  #include "audit.h"
>  
> @@ -1862,7 +1864,7 @@ void __audit_inode(struct filename *name, const struct 
> dentry *dentry,
>  
>  	list_for_each_entry_reverse(n, &context->names_list, list) {
>  		/* does the name pointer match? */
> -		if (!n->name || n->name->name != name->name)
> +		if (!n->name || strcmp(n->name->name, name->name))
>  			continue;
>  
>  		/* match the correct record type */
> @@ -1881,14 +1883,39 @@ out_alloc:
>  	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
>  	if (!n)
>  		return;
> -	if (name)
> -		/* since name is not NULL we know there is already a matching
> -		 * name record, see audit_getname(), so there must be a type
> -		 * mismatch; reuse the string path since the original name
> -		 * record will keep the string valid until we free it in
> -		 * audit_free_names() */
> -		n->name = name;
> +	/* unfortunately, while we may have a path name to record with the
> +	 * inode, we can't always rely on the string lasting until the end of
> +	 * the syscall so we need to create our own copy, it may fail due to
> +	 * memory allocation issues, but we do our best */
> +	if (name) {
> +		/* we can't use getname_kernel() due to size limits */
> +		struct filename *new = __getname();
>  
> +		if (unlikely(!new))
> +			goto out;
> +
> +		memset(new, 0, sizeof(*new));
> +		if ((strlen(name->name) + 1) <= (PATH_MAX - sizeof(*new))) {
> +			char *new_name = (char *)(new) + sizeof(*new);
> +			new->name = new_name;
> +			new->separate = false;
> +		} else {
> +			/* this looks odd, but is due to final_putname() */
> +			struct filename *new2;
> +			new2 = kzalloc(sizeof(*new2), GFP_KERNEL);
> +			if (unlikely(!new2)) {
> +				__putname(new);
> +				goto out;
> +			}
> +			new2->name = (char *)new;
> +			new = new2;
> +			new->separate = true;
> +		}
> +		strcpy((char *)new->name, name->name);
> +		new->aname = n;
> +		n->name = new;
> +		n->name_put = true;
> +	}
>  out:
>  	if (parent) {
>  		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;
> 

  n22kvm-clone linux patch -p1 --dry-run < /mnt/t44/devel/kvm.patch
patching file kernel/auditsc.c
patch: **** malformed patch at line 15: dentry *dentry,

-- 
Toralf
pgp key: 7B1A 07F4 EC82 0F90 D4C2  8936 872A E508 0076 E94E

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Toralf Förster]

On 12/30/2014 07:46 PM, Paul Moore wrote:
>  
> @@ -1862,7 +1864,7 @@ void __audit_inode(struct filename *name, const struct 
> dentry *dentry,
>  
repairing this line and applying it to 3.19-rc2 of the KVM seems to solve the issue

Thx

-- 
Toralf
pgp key: 7B1A 07F4 EC82 0F90 D4C2  8936 872A E508 0076 E94E

Re: v3.19-rc2: crashes during boot (syslog-ng, rpcbind ...)

[Paul Moore]

On Wednesday, December 31, 2014 11:57:16 AM Toralf Förster wrote:
> On 12/30/2014 07:46 PM, Paul Moore wrote:
> > @@ -1862,7 +1864,7 @@ void __audit_inode(struct filename *name, const
> > struct dentry *dentry,
> 
> repairing this line and applying it to 3.19-rc2 of the KVM seems to solve
> the issue

Great, thanks for testing.  I'll post out a proper patch today, update the 
audit tree, and send a pull request to Linus.

-- 
paul moore
www.paul-moore.com