Re: [PATCH v13 3/9] open: O_EMPTYPATH: procfs-less file descriptor re-opening

2019-09-30 Thread Aleksa Sarai 
On 2019-10-01, kbuild test robot  wrote:
> Hi Aleksa,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [cannot apply to v5.4-rc1 next-20190930]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see 
> https://stackoverflow.com/a/37406982]
> 
> url:
> https://github.com/0day-ci/linux/commits/Aleksa-Sarai/namei-openat2-2-path-resolution-restrictions/20191001-025628
> config: sparc-allyesconfig (attached as .config)
> compiler: sparc64-linux-gcc (GCC) 7.4.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=sparc 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot 
> 
> All error/warnings (new ones prefixed by >>):
> 
>In file included from include/linux/kernel.h:11:0,
> from include/linux/list.h:9,
> from include/linux/wait.h:7,
> from include/linux/wait_bit.h:8,
> from include/linux/fs.h:6,
> from include/uapi/linux/aio_abi.h:31,
> from include/linux/syscalls.h:74,
> from fs/fcntl.c:8:
>fs/fcntl.c: In function 'fcntl_init':
> >> include/linux/compiler.h:350:38: error: call to 
> >> '__compiletime_assert_1037' declared with attribute error: BUILD_BUG_ON 
> >> failed: 22 - 1 != HWEIGHT32( (VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) 
> >> | __FMODE_EXEC | __FMODE_NONOTIFY)
>  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>  ^
>include/linux/compiler.h:331:4: note: in definition of macro 
> '__compiletime_assert'
>prefix ## suffix();\
>^~
>include/linux/compiler.h:350:2: note: in expansion of macro 
> '_compiletime_assert'
>  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>  ^~~
>include/linux/build_bug.h:39:37: note: in expansion of macro 
> 'compiletime_assert'
> #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> ^~
>include/linux/build_bug.h:50:2: note: in expansion of macro 
> 'BUILD_BUG_ON_MSG'
>  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>  ^~~~
> >> fs/fcntl.c:1034:2: note: in expansion of macro 'BUILD_BUG_ON'
>  BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
>  ^~~~

This is because 0x400 is used by FMODE_NONOTIFY. The fix is simple,
and I'll include it in the next version.

> vim +/__compiletime_assert_1037 +350 include/linux/compiler.h
> 
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  336  
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  337  #define 
> _compiletime_assert(condition, msg, prefix, suffix) \
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  338  
> __compiletime_assert(condition, msg, prefix, suffix)
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  339  
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  340  /**
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  341   * compiletime_assert - break 
> build and emit msg if condition is false
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  342   * @condition: a compile-time 
> constant condition to check
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  343   * @msg:   a message to 
> emit if condition is false
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  344   *
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  345   * In tradition of POSIX 
> assert, this macro will break the build if the
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  346   * supplied condition is 
> *false*, emitting the supplied error message if the
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  347   * compiler has support to do 
> so.
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  348   */
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  349  #define 
> compiletime_assert(condition, msg) \
> 9a8ab1c39970a4 Daniel Santos 2013-02-21 @350  
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> 9a8ab1c39970a4 Daniel Santos 2013-02-21  351  
> 
> :: The code at line 350 was first introduced by commit
> :: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce 
> compiletime_assert & BUILD_BUG_ON_MSG
> 
> :: TO: Daniel Santos 
> :: CC: Linus Torvalds 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

[PATCH v13 3/9] open: O_EMPTYPATH: procfs-less file descriptor re-opening

2019-09-30 Thread Aleksa Sarai 
Userspace has made use of /proc/self/fd very liberally to allow for
descriptors to be re-opened. There are a wide variety of uses for this
feature, but it has always required constructing a pathname and could
not be done without procfs mounted. The obvious solution for this is to
extend openat(2) to have an AT_EMPTY_PATH-equivalent -- O_EMPTYPATH.

Now that descriptor re-opening has been made safe through the new
magic-link resolution restrictions, we can replicate these restrictions
for O_EMPTYPATH. In particular, we only allow "upgrading" the file
descriptor if the corresponding FMODE_PATH_* bit is set (or the
FMODE_{READ,WRITE} cases for non-O_PATH file descriptors).

When doing openat(O_EMPTYPATH|O_PATH), O_PATH takes precedence and
O_EMPTYPATH is ignored. Very few users ever have a need to O_PATH
re-open an existing file descriptor, and so accommodating them at the
expense of further complicating O_PATH makes little sense. Ultimately,
if users ask for this we can always add RESOLVE_EMPTY_PATH to
resolveat(2) in the future.

Signed-off-by: Aleksa Sarai 
---
 arch/alpha/include/uapi/asm/fcntl.h  |  1 +
 arch/parisc/include/uapi/asm/fcntl.h | 39 ++--
 arch/sparc/include/uapi/asm/fcntl.h  |  1 +
 fs/fcntl.c   |  2 +-
 fs/namei.c   | 20 ++
 fs/open.c|  7 -
 include/linux/fcntl.h|  2 +-
 include/uapi/asm-generic/fcntl.h |  4 +++
 8 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/fcntl.h 
b/arch/alpha/include/uapi/asm/fcntl.h
index 50bdc8e8a271..1f879bade68b 100644
--- a/arch/alpha/include/uapi/asm/fcntl.h
+++ b/arch/alpha/include/uapi/asm/fcntl.h
@@ -34,6 +34,7 @@
 
 #define O_PATH 04000
 #define __O_TMPFILE01
+#define O_EMPTYPATH02
 
 #define F_GETLK7
 #define F_SETLK8
diff --git a/arch/parisc/include/uapi/asm/fcntl.h 
b/arch/parisc/include/uapi/asm/fcntl.h
index 03ce20e5ad7d..5d709058a76f 100644
--- a/arch/parisc/include/uapi/asm/fcntl.h
+++ b/arch/parisc/include/uapi/asm/fcntl.h
@@ -2,26 +2,27 @@
 #ifndef _PARISC_FCNTL_H
 #define _PARISC_FCNTL_H
 
-#define O_APPEND   00010
-#define O_BLKSEEK  00100 /* HPUX only */
-#define O_CREAT00400 /* not fcntl */
-#define O_EXCL 02000 /* not fcntl */
-#define O_LARGEFILE04000
-#define __O_SYNC   00010
+#define O_APPEND   10
+#define O_BLKSEEK  000100 /* HPUX only */
+#define O_CREAT000400 /* not fcntl */
+#define O_EXCL 002000 /* not fcntl */
+#define O_LARGEFILE004000
+#define __O_SYNC   10
 #define O_SYNC (__O_SYNC|O_DSYNC)
-#define O_NONBLOCK 00024 /* HPUX has separate NDELAY & NONBLOCK */
-#define O_NOCTTY   00040 /* not fcntl */
-#define O_DSYNC00100 /* HPUX only */
-#define O_RSYNC00200 /* HPUX only */
-#define O_NOATIME  00400
-#define O_CLOEXEC  01000 /* set close_on_exec */
-
-#define O_DIRECTORY1 /* must be a directory */
-#define O_NOFOLLOW 00200 /* don't follow links */
-#define O_INVISIBLE00400 /* invisible I/O, for DMAPI/XDSM */
-
-#define O_PATH 02000
-#define __O_TMPFILE04000
+#define O_NONBLOCK 24 /* HPUX has separate NDELAY & NONBLOCK */
+#define O_NOCTTY   40 /* not fcntl */
+#define O_DSYNC000100 /* HPUX only */
+#define O_RSYNC000200 /* HPUX only */
+#define O_NOATIME  000400
+#define O_CLOEXEC  001000 /* set close_on_exec */
+
+#define O_DIRECTORY01 /* must be a directory */
+#define O_NOFOLLOW 000200 /* don't follow links */
+#define O_INVISIBLE000400 /* invisible I/O, for DMAPI/XDSM */
+
+#define O_PATH 002000
+#define __O_TMPFILE004000
+#define O_EMPTYPATH01
 
 #define F_GETLK64  8
 #define F_SETLK64  9
diff --git a/arch/sparc/include/uapi/asm/fcntl.h 
b/arch/sparc/include/uapi/asm/fcntl.h
index 67dae75e5274..dc86c9eaf950 100644
--- a/arch/sparc/include/uapi/asm/fcntl.h
+++ b/arch/sparc/include/uapi/asm/fcntl.h
@@ -37,6 +37,7 @@
 
 #define O_PATH 0x100
 #define __O_TMPFILE0x200
+#define O_EMPTYPATH0x400
 
 #define F_GETOWN   5   /*  for sockets. */
 #define F_SETOWN   6   /*  for sockets. */
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 3d40771e8e7c..4cf05a2fd162 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1031,7 +1031,7 @@ static int __init fcntl_init(void)
 * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
 * is defined as O_NONBLOCK on some platforms and not on others.
 */
-   BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+   BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=