Bug 171380 - [ZFS] 9.1: Panic when importing pool from OpenSolaris/OpenIndiana that has multiple FUID domains
Summary: [ZFS] 9.1: Panic when importing pool from OpenSolaris/OpenIndiana that has mu...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 9.1-PRERELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Martin Matuska
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-06 14:30 UTC by Bryan Drewery
Modified: 2012-09-09 22:34 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan Drewery 2012-09-06 14:30:01 UTC
	I imported a ZFS pool from an OpenIndiana system. When I tried to traverse into a certain directory, the system would panic:

	    panic: avl_find() succeeded inside avl_add()
	    avl_add+0x4b
	    zfs_fuid_table_load+0x198
	    zfs_fuid_init+0x12c
	    zfs_fuid_find_by_idx+0xc7
	    zfs_fuid_map_id+0x19
	    zfs_groupmember+0x16
	    zfs_zaccess_aces_check+0x196
	    zfs_zaccess+0xc6
	    zfs_freebsd_getattr+0x1c1
	    ugidfw_check_vp+0x6c
	    mac_vnode_check_stat+0xa7
	    vn_stat+0x39
	    kern_statat_vnhook+0xf9
	    kern_statat+0x15
	    sys_stat+0x2a
	    amd64_syscall+0x540

	I disabled MAC/ugidfw and still had the issue:

	    panic: avl_find() succeeded inside avl_add()
	    avl_add+0x4b
	    zfs_fuid_table_load+0x198
	    zfs_fuid_init+0x12c
	    zfs_fuid_find_by_idx+0xc7
	    zfs_fuid_map_id+0x19
	    zfs_groupmember+0x16
	    zfs_zaccess_aces_check+0x196
	    zfs_zaccess+0xc6
	    zfs_freebsd_getattr+0x1c1
	    vn_stat+0x6a
	    kern_statat_vnhook+0xf9
	    kern_statat+0x15
	    sys_lstat+0x2a
	    amd64_syscall+0x540


	The code that is causing issue is with importing FUIDs that have different domains. The FreeBSD code hardcodes "FreeBSD" as the domain, so importing two objects with two different domains, results in "FreeBSD" added to the avl tree twice, resulting in the panic.

Fix: 

The panic was fixed by r230454 in HEAD on Jan 22 2012. MFCing it fixes it. After applying to my machine, I was able to traverse into these directories/files.
How-To-Repeat: 	I've tried several times to create a pool on OpenSolaris 111,134 and OpenIndiana 151 with ACLs and modifying the ACLs over CIFS. I'm pretty certain that CIFS-created files are a major factor here. None of these datasets results in the panic though. I admit I am unclear where this FUID domain comes from, so I am unable to properly produce a dataset needed to produce the issue.
Comment 1 Bryan Drewery freebsd_committer freebsd_triage 2012-09-06 14:32:12 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer
Comment 2 dfilter service freebsd_committer freebsd_triage 2012-09-06 14:44:01 UTC
Author: mm
Date: Thu Sep  6 13:43:48 2012
New Revision: 240162
URL: http://svn.freebsd.org/changeset/base/240162

Log:
  Make r230454 more readable and vendor-like.
  
  PR:		kern/171380
  MFC after:	3 days

Modified:
  head/sys/cddl/compat/opensolaris/sys/sid.h

Modified: head/sys/cddl/compat/opensolaris/sys/sid.h
==============================================================================
--- head/sys/cddl/compat/opensolaris/sys/sid.h	Thu Sep  6 10:10:56 2012	(r240161)
+++ head/sys/cddl/compat/opensolaris/sys/sid.h	Thu Sep  6 13:43:48 2012	(r240162)
@@ -30,7 +30,8 @@
 #define	_OPENSOLARIS_SYS_SID_H_
 
 typedef struct ksiddomain {
-	char	kd_name[1];	/* Domain part of SID */
+	char	*kd_name;	/* Domain part of SID */
+	uint_t	kd_len;
 } ksiddomain_t;
 typedef void	ksid_t;
 
@@ -38,8 +39,12 @@ static __inline ksiddomain_t *
 ksid_lookupdomain(const char *domain)
 {
 	ksiddomain_t *kd;
+	size_t len;
 
-	kd = kmem_alloc(sizeof(*kd) + strlen(domain), KM_SLEEP);
+	len = strlen(domain) + 1;
+	kd = kmem_alloc(sizeof(*kd), KM_SLEEP);
+	kd->kd_len = (uint_t)len;
+	kd->kd_name = kmem_alloc(len, KM_SLEEP);
 	strcpy(kd->kd_name, domain);
 	return (kd);
 }
@@ -48,6 +53,7 @@ static __inline void
 ksiddomain_rele(ksiddomain_t *kd)
 {
 
+	kmem_free(kd->kd_name, kd->kd_len);
 	kmem_free(kd, sizeof(*kd));
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 3 Martin Matuska freebsd_committer freebsd_triage 2012-09-06 14:48:59 UTC
Responsible Changed
From-To: freebsd-fs->mm

I'll take it.
Comment 4 Martin Matuska freebsd_committer freebsd_triage 2012-09-08 22:58:22 UTC
I have created a pool in OpenIndiana to reproduce this error.

To reproduce, download the following file:
http://people.freebsd.org/~mm/misc/ksiddomain_testpool.gz

Then execute the following steps (replace /some/directory with a testing
directory, e.g. /mnt):
gunzip ksiddomain_testpool.gz
mdconfig -a -t vnode -f ksiddomain_testpool
zpool import -o altroot=/some/directory test
ls /some/directory

On a unpatched system, this causes panic.

-- 
Martin Matuska
FreeBSD committer
http://blog.vx.sk
Comment 5 dfilter service freebsd_committer freebsd_triage 2012-09-09 08:38:36 UTC
Author: mm
Date: Sun Sep  9 07:38:15 2012
New Revision: 240258
URL: http://svn.freebsd.org/changeset/base/240258

Log:
  MFC r240162:
  Make r230454 more readable and vendor-like.
  
  PR:	kern/171380

Modified:
  stable/9/sys/cddl/compat/opensolaris/sys/sid.h
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/cddl/compat/opensolaris/sys/sid.h
==============================================================================
--- stable/9/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:23:15 2012	(r240257)
+++ stable/9/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:38:15 2012	(r240258)
@@ -30,7 +30,8 @@
 #define	_OPENSOLARIS_SYS_SID_H_
 
 typedef struct ksiddomain {
-	char	kd_name[1];	/* Domain part of SID */
+	char	*kd_name;	/* Domain part of SID */
+	uint_t	kd_len;
 } ksiddomain_t;
 typedef void	ksid_t;
 
@@ -38,8 +39,12 @@ static __inline ksiddomain_t *
 ksid_lookupdomain(const char *domain)
 {
 	ksiddomain_t *kd;
+	size_t len;
 
-	kd = kmem_alloc(sizeof(*kd) + strlen(domain), KM_SLEEP);
+	len = strlen(domain) + 1;
+	kd = kmem_alloc(sizeof(*kd), KM_SLEEP);
+	kd->kd_len = (uint_t)len;
+	kd->kd_name = kmem_alloc(len, KM_SLEEP);
 	strcpy(kd->kd_name, domain);
 	return (kd);
 }
@@ -48,6 +53,7 @@ static __inline void
 ksiddomain_rele(ksiddomain_t *kd)
 {
 
+	kmem_free(kd->kd_name, kd->kd_len);
 	kmem_free(kd, sizeof(*kd));
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 dfilter service freebsd_committer freebsd_triage 2012-09-09 08:47:35 UTC
Author: mm
Date: Sun Sep  9 07:46:45 2012
New Revision: 240259
URL: http://svn.freebsd.org/changeset/base/240259

Log:
  MFC r240162:
  Make r230454 more readable and vendor-like.
  
  PR:	kern/171380

Modified:
  stable/8/sys/cddl/compat/opensolaris/sys/sid.h
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/cddl/   (props changed)

Modified: stable/8/sys/cddl/compat/opensolaris/sys/sid.h
==============================================================================
--- stable/8/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:38:15 2012	(r240258)
+++ stable/8/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:46:45 2012	(r240259)
@@ -30,7 +30,8 @@
 #define	_OPENSOLARIS_SYS_SID_H_
 
 typedef struct ksiddomain {
-	char	kd_name[1];	/* Domain part of SID */
+	char	*kd_name;	/* Domain part of SID */
+	uint_t	kd_len;
 } ksiddomain_t;
 typedef void	ksid_t;
 
@@ -38,8 +39,12 @@ static __inline ksiddomain_t *
 ksid_lookupdomain(const char *domain)
 {
 	ksiddomain_t *kd;
+	size_t len;
 
-	kd = kmem_alloc(sizeof(*kd) + strlen(domain), KM_SLEEP);
+	len = strlen(domain) + 1;
+	kd = kmem_alloc(sizeof(*kd), KM_SLEEP);
+	kd->kd_len = (uint_t)len;
+	kd->kd_name = kmem_alloc(len, KM_SLEEP);
 	strcpy(kd->kd_name, domain);
 	return (kd);
 }
@@ -48,6 +53,7 @@ static __inline void
 ksiddomain_rele(ksiddomain_t *kd)
 {
 
+	kmem_free(kd->kd_name, kd->kd_len);
 	kmem_free(kd, sizeof(*kd));
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 dfilter service freebsd_committer freebsd_triage 2012-09-09 08:47:44 UTC
Author: mm
Date: Sun Sep  9 07:47:02 2012
New Revision: 240260
URL: http://svn.freebsd.org/changeset/base/240260

Log:
  MFC r240162:
  Make r230454 more readable and vendor-like.
  
  PR:	kern/171380

Modified:
  stable/7/sys/cddl/compat/opensolaris/sys/sid.h
Directory Properties:
  stable/7/sys/   (props changed)

Modified: stable/7/sys/cddl/compat/opensolaris/sys/sid.h
==============================================================================
--- stable/7/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:46:45 2012	(r240259)
+++ stable/7/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 07:47:02 2012	(r240260)
@@ -30,7 +30,8 @@
 #define	_OPENSOLARIS_SYS_SID_H_
 
 typedef struct ksiddomain {
-	char	kd_name[1];	/* Domain part of SID */
+	char	*kd_name;	/* Domain part of SID */
+	uint_t	kd_len;
 } ksiddomain_t;
 typedef void	ksid_t;
 
@@ -38,8 +39,12 @@ static __inline ksiddomain_t *
 ksid_lookupdomain(const char *domain)
 {
 	ksiddomain_t *kd;
+	size_t len;
 
-	kd = kmem_alloc(sizeof(*kd) + strlen(domain), KM_SLEEP);
+	len = strlen(domain) + 1;
+	kd = kmem_alloc(sizeof(*kd), KM_SLEEP);
+	kd->kd_len = (uint_t)len;
+	kd->kd_name = kmem_alloc(len, KM_SLEEP);
 	strcpy(kd->kd_name, domain);
 	return (kd);
 }
@@ -48,6 +53,7 @@ static __inline void
 ksiddomain_rele(ksiddomain_t *kd)
 {
 
+	kmem_free(kd->kd_name, kd->kd_len);
 	kmem_free(kd, sizeof(*kd));
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 dfilter service freebsd_committer freebsd_triage 2012-09-09 21:26:31 UTC
Author: mm
Date: Sun Sep  9 20:26:19 2012
New Revision: 240288
URL: http://svn.freebsd.org/changeset/base/240288

Log:
  MFC r230454 (pjd):
  Use provided name when allocating ksid domain. It isn't really used
  on FreeBSD, but should fix a panic when pool is imported from another OS
  that is using this.
  
  MFC r240162 (mm):
  Make r230454 more readable and vendor-like.
  
  PR:		kern/171380
  Approved by:	re (kib)

Modified:
  releng/9.1/sys/cddl/compat/opensolaris/sys/sid.h
Directory Properties:
  releng/9.1/sys/   (props changed)

Modified: releng/9.1/sys/cddl/compat/opensolaris/sys/sid.h
==============================================================================
--- releng/9.1/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 20:13:11 2012	(r240287)
+++ releng/9.1/sys/cddl/compat/opensolaris/sys/sid.h	Sun Sep  9 20:26:19 2012	(r240288)
@@ -30,7 +30,8 @@
 #define	_OPENSOLARIS_SYS_SID_H_
 
 typedef struct ksiddomain {
-	char	kd_name[16];	/* Domain part of SID */
+	char	*kd_name;	/* Domain part of SID */
+	uint_t	kd_len;
 } ksiddomain_t;
 typedef void	ksid_t;
 
@@ -38,9 +39,13 @@ static __inline ksiddomain_t *
 ksid_lookupdomain(const char *domain)
 {
 	ksiddomain_t *kd;
+	size_t len;
 
+	len = strlen(domain) + 1;
 	kd = kmem_alloc(sizeof(*kd), KM_SLEEP);
-	strlcpy(kd->kd_name, "FreeBSD", sizeof(kd->kd_name));
+	kd->kd_len = (uint_t)len;
+	kd->kd_name = kmem_alloc(len, KM_SLEEP);
+	strcpy(kd->kd_name, domain);
 	return (kd);
 }
 
@@ -48,6 +53,7 @@ static __inline void
 ksiddomain_rele(ksiddomain_t *kd)
 {
 
+	kmem_free(kd->kd_name, kd->kd_len);
 	kmem_free(kd, sizeof(*kd));
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 Martin Matuska freebsd_committer freebsd_triage 2012-09-09 22:34:29 UTC
State Changed
From-To: open->closed

Resolved. Thanks!