From owner-p4-projects@FreeBSD.ORG Mon Aug 11 19:54:10 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0D5871065682; Mon, 11 Aug 2008 19:54:10 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C41881065678 for ; Mon, 11 Aug 2008 19:54:09 +0000 (UTC) (envelope-from trasz@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id B1A848FC1C for ; Mon, 11 Aug 2008 19:54:09 +0000 (UTC) (envelope-from trasz@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.2/8.14.2) with ESMTP id m7BJs9Ec066410 for ; Mon, 11 Aug 2008 19:54:09 GMT (envelope-from trasz@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.2/8.14.1/Submit) id m7BJs9QX066408 for perforce@freebsd.org; Mon, 11 Aug 2008 19:54:09 GMT (envelope-from trasz@freebsd.org) Date: Mon, 11 Aug 2008 19:54:09 GMT Message-Id: <200808111954.m7BJs9QX066408@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to trasz@freebsd.org using -f From: Edward Tomasz Napierala To: Perforce Change Reviews Cc: Subject: PERFORCE change 147175 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Aug 2008 19:54:10 -0000 http://perforce.freebsd.org/chv.cgi?CH=147175 Change 147175 by trasz@trasz_traszkan on 2008/08/11 19:53:25 With NFS4 ACLs, chmod(2) may need to add additional entries. Make sure it has room for that by limiting the acl_cnt to about half of ACL_MAX_ENTRIES in ACL setting routines. This is similar to what SunOS does. Bump ACL_MAX_ENTRIES to 202, so that "struct acl" is exactly one page long. This breaks binary compatibility, requring libc recompilation, and on-disk format, requiring one to remove nfs4.acl extattr for every file. Affected files ... .. //depot/projects/soc2008/trasz_nfs4acl/TODO#40 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.h#8 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#20 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_acl.c#9 edit .. //depot/projects/soc2008/trasz_nfs4acl/sys/sys/acl.h#18 edit Differences ... ==== //depot/projects/soc2008/trasz_nfs4acl/TODO#40 (text+ko) ==== @@ -12,11 +12,6 @@ - Add the information about correct constants to the manual pages. -- Decide what to do when chmod(2) needs to add ACL entries, but - there is no room in 'struct acl' to do that. Solaris seems to - limit the numer of user-settable entries to half of ACL_MAX_ENTRIES, - so there is no risk of running out of them in chmod(2). - - Make 'struct acl' variable size. - Benchmark things. ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.h#8 (text+ko) ==== @@ -33,7 +33,7 @@ #define _ACL_SUPPORT_H #define _POSIX1E_ACL_STRING_PERM_MAXSIZE 3 /* read, write, exec */ -#define _ACL_T_ALIGNMENT_BITS 12 +#define _ACL_T_ALIGNMENT_BITS 13 int _acl_type_unold(acl_type_t type); int _acl_differs(const acl_t a, const acl_t b); ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#20 (text+ko) ==== @@ -98,6 +98,9 @@ if (denied_explicitly != NULL) *denied_explicitly = 0; + KASSERT(aclp->acl_cnt > 0, ("aclp->acl_cnt > 0")); + KASSERT(aclp->acl_cnt <= ACL_MAX_ENTRIES, ("aclp->acl_cnt <= ACL_MAX_ENTRIES")); + for (i = 0; i < aclp->acl_cnt; i++) { entry = &(aclp->acl_entry[i]); @@ -289,12 +292,12 @@ { struct acl_entry *entry; + KASSERT(aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES, + ("aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES")); + entry = &(aclp->acl_entry[aclp->acl_cnt]); aclp->acl_cnt++; - if (aclp->acl_cnt >= ACL_MAX_ENTRIES) - return (NULL); - entry->ae_tag = tag; entry->ae_id = ACL_UNDEFINED_ID; entry->ae_perm = perm; @@ -309,8 +312,8 @@ { int i; - if (aclp->acl_cnt + 1 >= ACL_MAX_ENTRIES) - return (NULL); + KASSERT(aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES, + ("aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES")); for (i = aclp->acl_cnt; i > entry_index; i--) aclp->acl_entry[i] = aclp->acl_entry[i - 1]; @@ -368,9 +371,6 @@ * ACE. */ copy = _acl_duplicate_entry(aclp, i); - /* XXX: Is EPERM a good choice here? */ - if (copy == NULL) - return (EPERM); /* * 1.3.2. In the first ACE, the flag @@ -472,8 +472,6 @@ */ previous = entry; entry = _acl_duplicate_entry(aclp, i); - if (entry == NULL) - return (EPERM); /* Adjust counter, as we've just extended the ACL. */ i++; @@ -840,8 +838,8 @@ ACL_ENTRY_FILE_INHERIT)) == 0) continue; - KASSERT(child_aclp->acl_cnt < ACL_MAX_ENTRIES, - ("child_aclp->acl_cnt < ACL_MAX_ENTRIES")); + KASSERT(child_aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES, + ("child_aclp->acl_cnt + 1 <= ACL_MAX_ENTRIES")); child_aclp->acl_entry[child_aclp->acl_cnt] = *parent_entry; child_aclp->acl_cnt++; } @@ -901,8 +899,6 @@ * 2.D. Copy the original ACE into a second, adjacent ACE. */ copy = _acl_duplicate_entry(child_aclp, i); - if (copy == NULL) - return (EPERM); /* * 2.E. On the first ACE, ensure that ACL_ENTRY_ONLY_INHERIT @@ -970,7 +966,7 @@ * valid. There can be none of them too. Really. */ - if (acl->acl_cnt > ACL_MAX_ENTRIES || acl->acl_cnt < 0) + if (aclp->acl_cnt > ACL_MAX_ENTRIES || aclp->acl_cnt <= 0) return (EINVAL); for (i = 0; i < aclp->acl_cnt; i++) { ==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_acl.c#9 (text+ko) ==== @@ -206,6 +206,18 @@ error = copyin_acl(aclp, inkernelacl, type); if (error != 0) goto out_free; + + /* + * With NFS4 ACLs, chmod(2) may need to add additional entries. + * Make sure it has enough room for that - splitting every entry + * into two and appending "canonical six" entries at the end. + */ + if (type == ACL_TYPE_NFS4 && + inkernelacl->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2) { + error = ENOSPC; + goto out_free; + } + error = vn_start_write(vp, &mp, V_WAIT | PCATCH); if (error != 0) goto out_free; @@ -302,6 +314,18 @@ error = copyin_acl(aclp, inkernelacl, type); if (error != 0) goto out_free; + + /* + * With NFS4 ACLs, chmod(2) may need to add additional entries. + * Make sure it has enough room for that - splitting every entry + * into two and appending "canonical six" entries at the end. + */ + if (type == ACL_TYPE_NFS4 && + inkernelacl->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2) { + error = ENOSPC; + goto out_free; + } + error = VOP_ACLCHECK(vp, type_unold(type), inkernelacl, td->td_ucred, td); out_free: ==== //depot/projects/soc2008/trasz_nfs4acl/sys/sys/acl.h#18 (text+ko) ==== @@ -50,7 +50,12 @@ #define NFS4_ACL_EXTATTR_NAMESPACE EXTATTR_NAMESPACE_SYSTEM #define NFS4_ACL_EXTATTR_NAME "nfs4.acl" #define OLDACL_MAX_ENTRIES 32 -#define ACL_MAX_ENTRIES OLDACL_MAX_ENTRIES +/* + * With 204 entries, "struct acl" is exactly one page big. + * Note that with NFS4 ACLs, the maximum number of ACL entries one + * may set on file or directory is about half of ACL_MAX_ENTRIES. + */ +#define ACL_MAX_ENTRIES 204 /* * "struct oldacl" is used in compatibility ACL syscalls and for on-disk