Skip to contentSkip to conversation

fcntl.h: define AT_EACCESS

PATCH In review maximilian attems 5 participants 6 messages Apr 16, 2010 – Apr 19, 2010 File locking (flock() and fcntl()/lockf())

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fcntl.h: define AT_EACCESS

noticed on a klibc build of dash that someone had left out that def:
usr/dash/bltin/test.c:490: error: ‘AT_EACCESS’ undeclared (first use in thiction)

---
 include/linux/fcntl.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/fcntl.h +2 -0
index 8603740..8bb001d 100644
@@ -39,6 +39,8 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_EACCESS		0x200	/* Test access permitted for
+					   effective IDs, not real IDs.  */
 
 #ifdef __KERNEL__
 
Signature · 1 line
1.7.0.4
#1 lore raw
Quoted 17 lines · On Fri, 16 Apr 2010 05:08:00 +0200 maximilian attems <[email protected]> wrote:

noticed on a klibc build of dash that someone had left out that def:
usr/dash/bltin/test.c:490: error: ___AT_EACCESS___ undeclared (first use in thiction)

---
 include/linux/fcntl.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/fcntl.h +0 -0
index 8603740..8bb001d 100644

include/linux/fcntl.h line 44

Quoted 6 lines
include/linux/fcntl.h +2 -0
@@ -39,6 +39,8 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_EACCESS		0x200	/* Test access permitted for
+					   effective IDs, not real IDs.  */
 

I'm all confused.

The affects sys_faccesat(), yes? But sys_faccesat() never gets passed a `flags' argument so how does the behaviour which the FACCESSAT(2) manpage describes get implemented?

This patch doesn't actually change kernel behaviour, so how can setting AT_EACCESS change any syscall's actions?

It's a bit of a worry that the proposed value for AT_EACCESS duplicates AT_REMOVEDIR. I guess that, despite apeparances, they're different namespaces. Any thoughts on the implications of this?

#2 lore raw
Quoted 33 lines · Andrew Morton
Quoted 17 lines · On Fri, 16 Apr 2010 05:08:00 +0200 maximilian attems <[email protected]> wrote:

noticed on a klibc build of dash that someone had left out that def:
usr/dash/bltin/test.c:490: error: ___AT_EACCESS___ undeclared (first use in thiction)

---
 include/linux/fcntl.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/fcntl.h +0 -0
index 8603740..8bb001d 100644

include/linux/fcntl.h line 44

Quoted 6 lines
include/linux/fcntl.h +2 -0
@@ -39,6 +39,8 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_EACCESS		0x200	/* Test access permitted for
+					   effective IDs, not real IDs.  */
 

I'm all confused.

The affects sys_faccesat(), yes? But sys_faccesat() never gets passed a `flags' argument so how does the behaviour which the FACCESSAT(2) manpage describes get implemented?

Quoted 6 lines

This patch doesn't actually change kernel behaviour, so how can setting AT_EACCESS change any syscall's actions?

It's a bit of a worry that the proposed value for AT_EACCESS duplicates AT_REMOVEDIR. I guess that, despite apeparances, they're different namespaces. Any thoughts on the implications of this?

glibc fcntl.h defines AT_EACCESS in the same way as aboves patch, concerning the implementation, others should know better.

the dash code calling faccessat has the 4 params,
klibc faccessat had only 3 args, guess nobody had used it before.

the relevant dash code reads:
#ifdef HAVE_FACCESSAT
static int test_file_access(const char *path, int mode)

{
	return !faccessat(AT_FDCWD, path, mode, AT_EACCESS);
}

#else /* HAVE_FACCESSAT */

#3 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 2 lines · maximilian attems

the dash code calling faccessat has the 4 params,
klibc faccessat had only 3 args, guess nobody had used it before.

The function is implemented at userlevel. The kernel code has the same name but isn't a complete implementation. There is no point in defining the symbol in the kernel headers.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖ -----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvM1L0ACgkQ2ijCOnn/RHS1zgCggyyqLkOWD7dyOeLOiC314niu +MYAoKQVKnG3DSZii9lodwvIHkBIppEF
=y6bR
-----END PGP SIGNATURE-----

#4 lore raw
Quoted 11 lines · Ulrich Drepper

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 2 lines · maximilian attems

the dash code calling faccessat has the 4 params,
klibc faccessat had only 3 args, guess nobody had used it before.

The function is implemented at userlevel. The kernel code has the same name but isn't a complete implementation. There is no point in defining the symbol in the kernel headers.

They should be added as a comment, at least, to avoid future conflicts.

-hpa

#5 lore raw
In reply to maximilian attems

Can you share some justification why it's worth extending faccessat() with new options?

Isn't faccessat() insecure in most use cases, due to TOCTTOU (time-of-check to time-of-use) vulnerabilities? When faccessat() returns 0, you learn that at some point in the past, the process had permission to access a given file, though the process may or may not have permission at the moment. Why is that a useful thing to know?

I'm sure you're familiar with all the standard arguments why using access() tends to represent a security vulnerability. Is there a reason why similar arguments do not apply to faccessat()?