[PATCH 0/1] Fix facl on files opened with O_PATH

[PATCH 0/1] Fix facl on files opened with O_PATH

[Ken Brown]

I'm not sure if this patch is right.  Should facl fail on all commands
or just on SETACL?  If the command is GETACL, for example, should this
fail like fgetxattr(2) or should it succeed like fstat(2)?

Cygwin may be the only platform that supports both facl(2) and O_PATH,
so I guess we're on our own here.

Ken Brown (1):
  Cygwin: facl: fail with EBADF on files opened with O_PATH

 winsup/cygwin/sec_acl.cc | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.30.0

[PATCH 1/1] Cygwin: facl: fail with EBADF on files opened with O_PATH

[Ken Brown]

This is in the spirit of the Linux requirement that file operations
like fchmod(2), fchown(2), and fgetxattr(2) fail with EBADF on files
opened with O_PATH.
---
 winsup/cygwin/sec_acl.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index fe015728d..90969b639 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -1246,6 +1246,11 @@ facl32 (int fd, int cmd, int nentries, aclent_t *aclbufp)
       syscall_printf ("-1 = facl (%d)", fd);
       return -1;
     }
+  if (cfd->get_flags () & O_PATH)
+    {
+      set_errno (EBADF);
+      return -1;
+    }
   int res = cfd->facl (cmd, nentries, aclbufp);
   syscall_printf ("%R = facl(%s) )", res, cfd->get_name ());
   return res;
-- 
2.30.0

Re: [PATCH 0/1] Fix facl on files opened with O_PATH

[Corinna Vinschen]

On Feb 23 17:49, Ken Brown via Cygwin-patches wrote:
> I'm not sure if this patch is right.  Should facl fail on all commands
> or just on SETACL?  If the command is GETACL, for example, should this
> fail like fgetxattr(2) or should it succeed like fstat(2)?
> 
> Cygwin may be the only platform that supports both facl(2) and O_PATH,
> so I guess we're on our own here.

Not entirely.  facl is also the underlying function for the POSIX ACL
calls, deprecated but still supported by Linux.  I. e., on a file system
supporting ACLs (xfs, ext4, etc), this needs testing:

  int fd = open (..., O_PATH);
  if (fd < 0)
    perror ("open");
  else if (acl_get_fd (fd) != NULL)
    printf ("acl_get_fd works with O_PATH\n");
  else
    perror ("acl_get_fd");

I just did that and it turns out that the above code returns with

  acl_get_fd: Bad file descriptor

At first I was actually a bit surprised.  I thought fetching ACLs is
along the lines of fstat, but on second though it's not.  ACLs are
stored as extended attributes and given that fgetxattr is supposed to
fail with EBADF, it's logical that acl_get_fd fails with EBADF as well.
qed

So your patch looks good.


Thanks,
Corinna