acl: Preserving the setuid/setgid/sticky bits

[Brandon Philips]

On 14:46 Wed 17 Jun 2009, Andreas Gruenbacher wrote:
> In the current version, getfacl only includes the new flags: comment for files 
> which have any of the special set. Setfacl --restore clears all special bits 
> if there is no flags: comment, and sets them accordingly otherwise. (Without 
> --restore, setfacl disregards such comments, just like it disregards the other 
> comments.)
> 
> Does this extension look reasonable?

Seems reasonable to me.

> Any objections to changing the behavior of --restore to clear the special 
> flags in case there is no flags: comment? (And if so, how would you like this 
> solved instead?)

Will need to put a block around the getfacl mode output in the
POSIXLY_CORRECT case. Other than that SUID/SGID/sticky restore if the
comment exists makes sense to me.

> @@ -467,19 +468,21 @@ do_set(
>  		goto cleanup;
>  	}
>  	if (acl) {
> +		mode_t mode = 0;
> +		int equiv_mode;
> +
> +		equiv_mode = acl_equiv_mode(acl, &mode);
> +
>  		if (acl_set_file(path_p, ACL_TYPE_ACCESS, acl) != 0) {
>  			if (errno == ENOSYS || errno == ENOTSUP) {
> -				int saved_errno = errno;
> -				mode_t mode;
> -
> -				if (acl_equiv_mode(acl, &mode) != 0) {
> -					errno = saved_errno;
> +				if (equiv_mode != 0)
>  					goto fail;
> -				} else if (chmod(path_p, mode) != 0)
> +				else if (chmod(path_p, mode) != 0)
>  					goto fail;
>  			} else
>  				goto fail;
>  		}
> +		args->mode = mode;
>  	}

These is a bit of a bug here. If the acl in do_set is the same as the
current acl then we will never reach here and mode will remain 0.

restore() from setfacl.c will OR this 0 mode with the flags from the
comment and chmod, obviously creating an incorrect result.

Here is the fix in restore()

New patch attached with this fix and a test case that will break with
the original patch applied.

+                       if (!args.mode)
+                               args.mode = st.st_mode;
+                       args.mode &= (S_IRWXU | S_IRWXG | S_IRWXO);
+                       if (chmod(path_p, flags | args.mode) != 0) {
+                               fprintf(stderr, _("%s: %s: Cannot change
"
+                                                 "mode: %s\n"),
+                                       progname, xquote(path_p,
"\n\r"),
+                                       strerror(errno));
+                               status = 1;
+                       }
+               }

Cheers,

	Brandon