xfs
[Top] [All Lists]

[PATCH] Re: chacl question

To: Federico Sevilla III <jijo@xxxxxxxxxxxxxxxxxxxx>
Subject: [PATCH] Re: chacl question
From: Andi Kleen <ak@xxxxxxx>
Date: Tue, 17 Jul 2001 16:55:43 +0200
Cc: Linux XFS Mailing List <linux-xfs@xxxxxxxxxxx>
In-reply-to: <Pine.LNX.4.21.0107172125090.467-100000@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>; from jijo@xxxxxxxxxxxxxxxxxxxx on Tue, Jul 17, 2001 at 09:29:01PM +0800
References: <Pine.LNX.4.21.0107172125090.467-100000@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: owner-linux-xfs@xxxxxxxxxxx
User-agent: Mutt/1.2.5i
On Tue, Jul 17, 2001 at 09:29:01PM +0800, Federico Sevilla III wrote:
> But it says:
> 
> chacl: "u::rwx,g::,o::,u:jijo:r-x,m::r-x" is an invalid ACL specification.

You cannot have multiple 'u' specifikations.


> 
> 'chacl -l test' output: test[] which (I think) verifies that ACLs are
> enabled and work.
> 
> What am I doing wrong? Also, is it possible to just add an ACL? Let's say
> I already have ACLs set up, and just want to enable someone else to read
> and access this file, how do I do this? Do I have to list everyone else?
> Or if I want to revoke access from someone?
> 
> It's my first time to handle ACLs. I hope you guys can be patient with
> this newbie. :) :)

Similar problems have also annoyed, annoyed me enough to produce the following
patch which adds somewhat usable error reporting to libacl.

Just apply it below cmd/chacl and get at least usable error messages.

It also fixes one bug in the manpage.


-Andi


Index: chacl/chacl.c
===================================================================
RCS file: /cvs/linux-2.4-xfs/cmd/acl/chacl/chacl.c,v
retrieving revision 1.5
diff -u -u -r1.5 chacl.c
--- chacl/chacl.c       2001/06/11 20:48:59     1.5
+++ chacl/chacl.c       2001/07/17 14:50:52
@@ -165,7 +165,11 @@
                acl = acl_from_text (argv[optind]);
                if (acl == NULL || acl_valid(acl) == -1)
                {
+                       char *errs;
+                       errs = acl_error_string(); 
                        fprintf (stderr, inv_acl, program, argv[optind]);
+                       if (errs) 
+                               fprintf (stderr, "%s\n", errs);
                        return (1);
                } 
                optind++;
Index: include/acl.h
===================================================================
RCS file: /cvs/linux-2.4-xfs/cmd/acl/include/acl.h,v
retrieving revision 1.6
diff -u -u -r1.6 acl.h
--- include/acl.h       2001/06/06 04:55:01     1.6
+++ include/acl.h       2001/07/17 14:50:52
@@ -163,6 +163,7 @@
 extern char *acl_to_short_text(acl_t, ssize_t *);
 extern char *acl_to_text(acl_t, ssize_t *);
 extern int acl_valid(acl_t);
+extern char *acl_error_string(void);
 
 /* system calls */
 extern int acl_get(const char *, int, struct acl *, struct acl *);
Index: libacl/acl.c
===================================================================
RCS file: /cvs/linux-2.4-xfs/cmd/acl/libacl/acl.c,v
retrieving revision 1.12
diff -u -u -r1.12 acl.c
--- libacl/acl.c        2001/07/02 03:31:14     1.12
+++ libacl/acl.c        2001/07/17 14:50:54
@@ -58,6 +58,18 @@
 
 #define setoserror(E)  errno = (E)
 
+#ifdef __GNUC__
+#define PRINTFLIKE(a,b) __attribute((format(printf, a, b)))
+#else
+#define PRINTFLIKE(a,b)
+#endif
+static void 
+acl_verbose_error (void *b0, void *b1, int e, char *fmt, ...) PRINTFLIKE(4,5);
+
+
+/* Not threadsafe, but overwriting is harmless */ 
+static char acl_error_buf[256];
+
 /* 
  * Compatibility flag for IRIX functionality
  * Default is to support common Linux/Posix ACL functionality
@@ -142,17 +154,33 @@
        return 0;
 }
 
+
 static void
 acl_error (void *b0, void *b1, int e)
 {
-
        if (b0)
                free (b0);
        if (b1)
                free (b1);
        setoserror (e);
+       acl_error_buf[0] = 0;
 }
 
+
+static void 
+acl_verbose_error (void *b0, void *b1, int e, char *fmt, ...) 
+{ 
+       va_list ap;
+       va_start(ap,fmt);
+       vsnprintf(acl_error_buf, sizeof(acl_error_buf), fmt, ap);
+       va_end(ap);
+       if (b0)
+               free (b0);
+       if (b1)
+               free (b1);
+       setoserror (e);
+} 
+
 /* 
  * Converts either long or short text form of ACL into internal representations
  *      Input long text form lines are either
@@ -173,8 +201,8 @@
        char c;
 
        /* check args for bogosity */
-       if (buf_p == NULL || *buf_p == '\0') {
-               acl_error (NULL, NULL, EINVAL);
+       if (buf_p == NULL || *buf_p == '\0') {                
+               acl_verbose_error (NULL, NULL, EINVAL, "zero or empty acl 
string");
                return (NULL);
        }
 
@@ -210,7 +238,7 @@
                char *tag, *qa, *perm;
 
                if (aclbuf->acl_cnt > ACL_MAX_ENTRIES) {
-                       acl_error (bp, aclbuf, EINVAL);
+                       acl_verbose_error (bp, aclbuf, EINVAL, "too many acl 
entries");
                        return (NULL);
                }
 
@@ -223,7 +251,7 @@
 
                /* get qualifier */
                if ((qa = skip_separator(fp)) == NULL) {
-                       acl_error (bp, aclbuf, EINVAL);
+                       acl_verbose_error (bp, aclbuf, EINVAL, "cannot separate 
qualifier after char %d(%4s)", fp-bp, fp);
                        return (NULL);
                }
                if (*qa == ':') {
@@ -238,7 +266,7 @@
 
                /* get permissions */
                if ((perm = skip_separator(fp)) == NULL) {
-                       acl_error (bp, aclbuf, EINVAL);
+                       acl_verbose_error (bp, aclbuf, EINVAL, "cannot separate 
 permissions after char %d(%4s)", fp-bp, fp);
                        return (NULL);
                }
                fp = skip_to_white(perm);
@@ -254,13 +282,22 @@
                        if (qa == NULL || *qa == '\0')
                                entry->ae_tag = ACL_USER_OBJ;
                        else {
+                               int olderrno = errno, e;
                                entry->ae_tag = ACL_USER;
-                               if ((pw = getpwnam(qa)))
+                               
+                               errno = 0; 
+                               pw = getpwnam(qa); 
+                               e = errno; 
+                               errno = olderrno;
+                               if (!pw && e) {
+                                       
acl_verbose_error(bp,aclbuf,EINVAL,"cannot find user %s: %s", qa, strerror(e));
+                                       return NULL;
+                               } else if (pw)
                                        entry->ae_id = pw->pw_uid;
                                else if (isdigit(*qa))
                                        entry->ae_id = atoi(qa);
                                else {
-                                       acl_error (bp, aclbuf, EINVAL);
+                                       acl_verbose_error (bp, aclbuf, EINVAL, 
"cannot parse user id %s", qa);
                                        return (NULL);
                                }
                        }
@@ -271,13 +308,22 @@
                        if (qa == NULL || *qa == '\0')
                                entry->ae_tag = ACL_GROUP_OBJ;
                        else {
+                               int olderrno=errno, e;
                                entry->ae_tag = ACL_GROUP;
-                               if ((gr = getgrnam (qa)))
+                               errno = 0; 
+                               gr = getgrnam(qa);
+                               e = errno;
+                               errno = olderrno; 
+                               if (gr)
                                        entry->ae_id = gr->gr_gid;
-                               else if (isdigit (*qa))
+                               else if (e) {
+                                       acl_verbose_error(bp,aclbuf,EINVAL,
+                                                         "cannot find group 
%s:%s\n", qa, strerror(e));
+                                       return NULL;
+                               } else if (isdigit (*qa))
                                        entry->ae_id = atoi(qa);
                                else {
-                                       acl_error (bp, aclbuf, EINVAL);
+                                       acl_verbose_error (bp, aclbuf, EINVAL, 
"invalid group %s", qa);
                                        return (NULL);
                                }
                        }
@@ -287,7 +333,7 @@
                if (!strcmp(tag, "other") || !strcmp(tag, "o")) {
                        entry->ae_tag = ACL_OTHER_OBJ;
                        if (qa != NULL && *qa != '\0') {
-                               acl_error (bp, aclbuf, EINVAL);
+                               acl_verbose_error (bp, aclbuf, EINVAL, "garbage 
after other: %s", qa);
                                return (NULL);
                        }
                }
@@ -296,19 +342,19 @@
                if (!strcmp(tag, "mask") || !strcmp(tag, "m")) {
                        entry->ae_tag = ACL_MASK;
                        if (qa != NULL && *qa != '\0') {
-                               acl_error (bp, aclbuf, EINVAL);
+                               acl_verbose_error (bp, aclbuf, EINVAL, "garbage 
after mask: %s", qa);
                                return (NULL);
                        }
                }
 
                /* Process invalid tag keyword */
                if (entry->ae_tag == -1) {
-                       acl_error(bp, aclbuf, EINVAL);
+                       acl_verbose_error(bp, aclbuf, EINVAL, "invalid tag");
                        return (NULL);
                }
 
                if (get_perm (perm, &entry->ae_perm) == -1) {
-                       acl_error ((void *) bp, (void *) aclbuf, EINVAL);
+                       acl_verbose_error ((void *) bp, (void *) aclbuf, 
EINVAL, "bad permission %s", perm);
                        return (NULL);
                }
        }
@@ -512,14 +558,18 @@
        int user = 0, group = 0, other = 0, mask = 0, mask_required = 0;
        int i, j;
 
-       if (aclp == NULL)
+       if (aclp == NULL) { 
+               acl_verbose_error(NULL,NULL,EINVAL,"no acl");
                goto acl_invalid;
-
+       }
+               
        if (aclp->acl_cnt == ACL_NOT_PRESENT)
                return 0;
 
-       if (aclp->acl_cnt < 0 || aclp->acl_cnt > ACL_MAX_ENTRIES)
+       if (aclp->acl_cnt < 0 || aclp->acl_cnt > ACL_MAX_ENTRIES) {
+               acl_verbose_error(NULL,NULL,EINVAL,"too many acl entries %d", 
aclp->acl_cnt);
                goto acl_invalid;
+       }
 
        for (i = 0; i < aclp->acl_cnt; i++)
        {
@@ -529,39 +579,72 @@
                switch (entry->ae_tag)
                {
                        case ACL_USER_OBJ:
-                               if (user++)
+                               if (user++) {
+                                       
acl_verbose_error(NULL,NULL,EINVAL,"multiple user entries");
                                        goto acl_invalid;
+                               }
                                break;
                        case ACL_GROUP_OBJ:
-                               if (group++)
+                               if (group++) {
+                                       
acl_verbose_error(NULL,NULL,EINVAL,"multiple group entries");
                                        goto acl_invalid;
+                               }
                                break;
                        case ACL_OTHER_OBJ:
-                               if (other++)
+                               if (other++){ 
+                                       
acl_verbose_error(NULL,NULL,EINVAL,"multiple other entries");
                                        goto acl_invalid;
+                               }
                                break;
                        case ACL_USER:
                        case ACL_GROUP:
                                for (j = i + 1; j < aclp->acl_cnt; j++)
                                {
                                        e = &aclp->acl_entry[j];
-                                       if (e->ae_id == entry->ae_id && 
e->ae_tag == entry->ae_tag)
+                                       if (e->ae_id == entry->ae_id && 
e->ae_tag == entry->ae_tag) {
+                                               char *what = 
entry->ae_tag==ACL_USER ? "user":"group";
+                                               acl_verbose_error(NULL,NULL,
+                             EINVAL,"duplicate %s acl for %s %d",
+                                                                 what,
+                                                                 what,
+                                                                 e->ae_id);
+                                                                 
                                                goto acl_invalid;
+                                       }
                                }
                                mask_required++;
                                break;
                        case ACL_MASK:
-                               if (mask++)
+                               if (mask++) { 
+                                       
acl_verbose_error(NULL,NULL,EINVAL,"multiple masks specified"); 
                                        goto acl_invalid;
+                               }
                                break;
                        default:
                                goto acl_invalid;
                }
        }
-       if (!user || !group || !other || (mask_required && !mask))
+       if (!user) {
+               acl_verbose_error(NULL,NULL,EINVAL,"missing user rights entry 
in acl");
                goto acl_invalid;
-       else
-               return 0;
+       }
+       if (!other) {
+               acl_verbose_error(NULL,NULL,EINVAL,"missing other rights entry 
in acl");
+               goto acl_invalid;
+       }
+       if (!group) {
+               acl_verbose_error(NULL,NULL,EINVAL,"missing group entry in 
acl");
+               goto acl_invalid;
+       }
+       if (!mask) {
+               acl_verbose_error(NULL,NULL,EINVAL,"missing mask in acl");
+               goto acl_invalid;
+       }
+       if (mask_required && !mask) { 
+               acl_verbose_error(NULL,NULL,EINVAL,"mask required and no mask");
+               goto acl_invalid;
+       }
+       return 0;
 acl_invalid:
        setoserror (EINVAL);
        return (-1);
@@ -1184,4 +1267,11 @@
                        "for this architecture\n");
        return 0;
 #endif
+}
+
+char *acl_error_string(void)
+{
+       if (acl_error_buf[0])
+               return acl_error_buf;
+       return NULL;
 }
Index: man/man5/acl.5
===================================================================
RCS file: /cvs/linux-2.4-xfs/cmd/acl/man/man5/acl.5,v
retrieving revision 1.4
diff -u -u -r1.4 acl.5
--- man/man5/acl.5      2001/06/15 14:25:43     1.4
+++ man/man5/acl.5      2001/07/17 14:50:55
@@ -100,8 +100,7 @@
 "user", "group", "other", "mask", "u", "g", "o", "m".
 .P
 The second field is a user name, numeric UID, group name, or numeric GID, 
depending
-on the value of the first field.  (\fIacl_from_text\fP(3c) supports only
-the strings, not the numeric UID/GID values.)
+on the value of the first field.  
 If the second field is empty, it implies
 that the ACL entry is for the owning user or group of the file.  Mask and
 other entries must have an empty second field.  The third field is


<Prev in Thread] Current Thread [Next in Thread>