Mercurial > hg > octave-lyh > gnulib-hg
changeset 16382:cc05dad27529
acl: Don't use GETACLCNT and similar ops, since they are unreliable.
- There were several instances of this pattern:
for (;;) {
n = acl (f, GETACLCNT, 0, NULL);
[ allocate an array A of size N ]
if (acl (f, GETACL, n, a) == n)
break;
}
This loop might never terminate if some other process is constantly
manipulating the file's ACL. The loop should be rewritten to
terminate.
- The acl (... GETACLNT ...) call is merely an optimization; its value
is merely a hint as to how big to make the array. A better
optimization is to avoid the acl (... GETACLNT ...) call entirely,
and just guess a reasonably-big size, growing the size and trying
again if it's not large enough. This guarantees termination, and
saves a system call.
* lib/acl-internal.h: Include <limits.h>.
(MIN, SIZE_MAX): New macros.
* lib/file-has-acl.c (file_has_acl) [Solaris]: Read the entries into
a stack-allocated buffer, and use malloc if it does not fit. Don't
use GETACLCNT.
* lib/set-mode-acl.c (qset_acl) [Solaris]: Likewise.
author | Paul Eggert <eggert@cs.ucla.edu> |
---|---|
date | Mon, 20 Feb 2012 01:12:06 +0100 |
parents | fc5c37ccbece |
children | 531aa00a1e80 |
files | ChangeLog lib/acl-internal.h lib/file-has-acl.c lib/set-mode-acl.c |
diffstat | 4 files changed, 200 insertions(+), 115 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,35 @@ +2012-02-19 Paul Eggert <eggert@cs.ucla.edu> + Bruno Haible <bruno@clisp.org> + + acl: Don't use GETACLCNT and similar ops, since they are unreliable. + + - There were several instances of this pattern: + + for (;;) { + n = acl (f, GETACLCNT, 0, NULL); + [ allocate an array A of size N ] + if (acl (f, GETACL, n, a) == n) + break; + } + + This loop might never terminate if some other process is constantly + manipulating the file's ACL. The loop should be rewritten to + terminate. + + - The acl (... GETACLNT ...) call is merely an optimization; its value + is merely a hint as to how big to make the array. A better + optimization is to avoid the acl (... GETACLNT ...) call entirely, + and just guess a reasonably-big size, growing the size and trying + again if it's not large enough. This guarantees termination, and + saves a system call. + + * lib/acl-internal.h: Include <limits.h>. + (MIN, SIZE_MAX): New macros. + * lib/file-has-acl.c (file_has_acl) [Solaris]: Read the entries into + a stack-allocated buffer, and use malloc if it does not fit. Don't + use GETACLCNT. + * lib/set-mode-acl.c (qset_acl) [Solaris]: Likewise. + 2012-02-19 Bruno Haible <bruno@clisp.org> acl: Fix endless loop on Solaris with vxfs.
--- a/lib/acl-internal.h +++ b/lib/acl-internal.h @@ -55,6 +55,15 @@ # define ENOTSUP (-1) #endif +#include <limits.h> +#ifndef MIN +# define MIN(a,b) ((a) < (b) ? (a) : (b)) +#endif + +#ifndef SIZE_MAX +# define SIZE_MAX ((size_t) -1) +#endif + #ifndef HAVE_FCHMOD # define HAVE_FCHMOD false # define fchmod(fd, mode) (-1)
--- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -556,7 +556,7 @@ return ACL_NOT_WELL_SUPPORTED (errno) ? 0 : -1; return ret; -# elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */ +# elif HAVE_FACL && defined GETACL /* Solaris, Cygwin, not HP-UX */ # if defined ACL_NO_TRIVIAL @@ -570,77 +570,135 @@ /* Solaris 2.5 through Solaris 10, Cygwin, and contemporaneous versions of Unixware. The acl() call returns the access and default ACL both at once. */ - int count; { - aclent_t *entries; + /* Initially, try to read the entries into a stack-allocated buffer. + Use malloc if it does not fit. */ + enum + { + alloc_init = 4000 / sizeof (aclent_t), /* >= 3 */ + alloc_max = MIN (INT_MAX, SIZE_MAX / sizeof (aclent_t)) + }; + aclent_t buf[alloc_init]; + size_t alloc = alloc_init; + aclent_t *entries = buf; + aclent_t *malloced = NULL; + int count; for (;;) { - count = acl (name, GETACLCNT, 0, NULL); - - if (count < 0) + count = acl (name, GETACL, alloc, entries); + if (count < 0 && errno == ENOSPC) { - if (errno == ENOSYS || errno == ENOTSUP) - break; - else - return -1; + /* Increase the size of the buffer. */ + free (malloced); + if (alloc > alloc_max / 2) + { + errno = ENOMEM; + return -1; + } + alloc = 2 * alloc; /* <= alloc_max */ + entries = malloced = + (aclent_t *) malloc (alloc * sizeof (aclent_t)); + if (entries == NULL) + { + errno = ENOMEM; + return -1; + } + continue; } - - if (count == 0) - break; - + break; + } + if (count < 0) + { + if (errno == ENOSYS || errno == ENOTSUP) + ; + else + { + int saved_errno = errno; + free (malloced); + errno = saved_errno; + return -1; + } + } + else if (count == 0) + ; + else + { /* Don't use MIN_ACL_ENTRIES: It's set to 4 on Cygwin, but Cygwin returns only 3 entries for files with no ACL. But this is safe: If there are more than 4 entries, there cannot be only the "user::", "group::", "other:", and "mask:" entries. */ if (count > 4) - return 1; - - entries = (aclent_t *) malloc (count * sizeof (aclent_t)); - if (entries == NULL) - { - errno = ENOMEM; - return -1; - } - if (acl (name, GETACL, count, entries) == count) { - if (acl_nontrivial (count, entries)) - { - free (entries); - return 1; - } - free (entries); - break; + free (malloced); + return 1; } - /* Huh? The number of ACL entries changed since the last call. - Repeat. */ - free (entries); + + if (acl_nontrivial (count, entries)) + { + free (malloced); + return 1; + } } + free (malloced); } # ifdef ACE_GETACL /* Solaris also has a different variant of ACLs, used in ZFS and NFSv4 file systems (whereas the other ones are used in UFS file systems). */ { - ace_t *entries; + /* Initially, try to read the entries into a stack-allocated buffer. + Use malloc if it does not fit. */ + enum + { + alloc_init = 4000 / sizeof (ace_t), /* >= 3 */ + alloc_max = MIN (INT_MAX, SIZE_MAX / sizeof (ace_t)) + }; + ace_t buf[alloc_init]; + size_t alloc = alloc_init; + ace_t *entries = buf; + ace_t *malloced = NULL; + int count; for (;;) { - int ret; - - count = acl (name, ACE_GETACLCNT, 0, NULL); - - if (count < 0) + count = acl (name, ACE_GETACL, alloc, entries); + if (count < 0 && errno == ENOSPC) { - if (errno == ENOSYS || errno == EINVAL) - break; - else - return -1; + /* Increase the size of the buffer. */ + free (malloced); + if (alloc > alloc_max / 2) + { + errno = ENOMEM; + return -1; + } + alloc = 2 * alloc; /* <= alloc_max */ + entries = malloced = (ace_t *) malloc (alloc * sizeof (ace_t)); + if (entries == NULL) + { + errno = ENOMEM; + return -1; + } + continue; } - - if (count == 0) - break; - + break; + } + if (count < 0) + { + if (errno == ENOSYS || errno == EINVAL) + ; + else + { + int saved_errno = errno; + free (malloced); + errno = saved_errno; + return -1; + } + } + else if (count == 0) + ; + else + { /* In the old (original Solaris 10) convention: If there are more than 3 entries, there cannot be only the ACE_OWNER, ACE_GROUP, ACE_OTHER entries. @@ -650,37 +708,18 @@ NEW_ACE_ACCESS_ALLOWED_ACE_TYPE and once with NEW_ACE_ACCESS_DENIED_ACE_TYPE. */ if (count > 6) - return 1; - - entries = (ace_t *) malloc (count * sizeof (ace_t)); - if (entries == NULL) - { - errno = ENOMEM; - return -1; - } - ret = acl (name, ACE_GETACL, count, entries); - if (ret < 0) { - free (entries); - if (errno == ENOSYS || errno == EINVAL) - break; - else - return -1; + free (malloced); + return 1; } - if (ret == count) + + if (acl_ace_nontrivial (count, entries)) { - if (acl_ace_nontrivial (count, entries)) - { - free (entries); - return 1; - } - free (entries); - break; + free (malloced); + return 1; } - /* Huh? The number of ACL entries changed since the last call. - Repeat. */ - free (entries); } + free (malloced); } # endif
--- a/lib/set-mode-acl.c +++ b/lib/set-mode-acl.c @@ -197,7 +197,7 @@ return chmod_or_fchmod (name, desc, mode); # endif -# elif HAVE_FACL && defined GETACLCNT /* Solaris, Cygwin, not HP-UX */ +# elif HAVE_FACL && defined GETACL /* Solaris, Cygwin, not HP-UX */ int done_setacl = 0; @@ -214,55 +214,60 @@ int convention; { + /* Initially, try to read the entries into a stack-allocated buffer. + Use malloc if it does not fit. */ + enum + { + alloc_init = 4000 / sizeof (ace_t), /* >= 3 */ + alloc_max = MIN (INT_MAX, SIZE_MAX / sizeof (ace_t)) + }; + ace_t buf[alloc_init]; + size_t alloc = alloc_init; + ace_t *entries = buf; + ace_t *malloced = NULL; int count; - ace_t *entries; for (;;) { - int ret; - - if (desc != -1) - count = facl (desc, ACE_GETACLCNT, 0, NULL); - else - count = acl (name, ACE_GETACLCNT, 0, NULL); - if (count <= 0) - { - convention = -1; - break; - } - entries = (ace_t *) malloc (count * sizeof (ace_t)); - if (entries == NULL) - { - errno = ENOMEM; - return -1; - } - ret = (desc != -1 - ? facl (desc, ACE_GETACL, count, entries) - : acl (name, ACE_GETACL, count, entries)); - if (ret < 0) + count = (desc != -1 + ? facl (desc, ACE_GETACL, alloc, entries) + : acl (name, ACE_GETACL, alloc, entries)); + if (count < 0 && errno == ENOSPC) { - free (entries); - convention = -1; - break; + /* Increase the size of the buffer. */ + free (malloced); + if (alloc > alloc_max / 2) + { + errno = ENOMEM; + return -1; + } + alloc = 2 * alloc; /* <= alloc_max */ + entries = malloced = (ace_t *) malloc (alloc * sizeof (ace_t)); + if (entries == NULL) + { + errno = ENOMEM; + return -1; + } + continue; } - if (ret == count) - { - int i; + break; + } - convention = 0; - for (i = 0; i < count; i++) - if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER)) - { - convention = 1; - break; - } - free (entries); - break; - } - /* Huh? The number of ACL entries changed since the last call. - Repeat. */ - free (entries); + if (count <= 0) + convention = -1; + else + { + int i; + + convention = 0; + for (i = 0; i < count; i++) + if (entries[i].a_flags & (OLD_ACE_OWNER | OLD_ACE_GROUP | OLD_ACE_OTHER)) + { + convention = 1; + break; + } } + free (malloced); } if (convention >= 0)