PERFORCE change 147175 for review
Edward Tomasz Napierala
trasz at FreeBSD.org
Mon Aug 11 19:54:10 UTC 2008
http://perforce.freebsd.org/chv.cgi?CH=147175
Change 147175 by trasz at 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
More information about the p4-projects
mailing list