[v2,02/17] y2038: Remove newstat family from default syscall set

Message ID 20180716161103.16239-3-arnd@arndb.de
State New
Headers show
Series
  • y2038: system calls, part 3
Related show

Commit Message

Arnd Bergmann July 16, 2018, 4:10 p.m.
We have four generations of stat() syscalls:
- the oldstat syscalls that are only used on the older architectures
- the newstat family that is used on all 64-bit architectures but
  lacked support for large files on 32-bit architectures.
- the stat64 family that is used mostly on 32-bit architectures to
  replace newstat
- statx() to replace all of the above, adding 64-bit timestamps among
  other things.

We already compile stat64 only on those architectures that need it,
but newstat is always built, including on those that don't reference
it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of
__ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of
newstat. All architectures that need it use an explict define, the
others now get a little bit smaller, and future architecture (including
64-bit targets) won't ever see it.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/alpha/include/asm/unistd.h      | 1 +
 arch/arm/include/asm/unistd.h        | 1 +
 arch/arm64/include/uapi/asm/unistd.h | 1 +
 arch/ia64/include/asm/unistd.h       | 2 ++
 arch/m68k/include/asm/unistd.h       | 1 +
 arch/microblaze/include/asm/unistd.h | 1 +
 arch/mips/include/asm/unistd.h       | 1 +
 arch/parisc/include/asm/unistd.h     | 1 +
 arch/powerpc/include/asm/unistd.h    | 1 +
 arch/s390/include/asm/unistd.h       | 1 +
 arch/sh/include/asm/unistd.h         | 1 +
 arch/sparc/include/asm/unistd.h      | 1 +
 arch/x86/include/asm/unistd.h        | 1 +
 arch/xtensa/include/asm/unistd.h     | 1 +
 fs/stat.c                            | 3 +++
 15 files changed, 18 insertions(+)

-- 
2.9.0

Comments

Christoph Hellwig July 17, 2018, 12:50 p.m. | #1
On Mon, Jul 16, 2018 at 06:10:48PM +0200, Arnd Bergmann wrote:
> We have four generations of stat() syscalls:

> - the oldstat syscalls that are only used on the older architectures

> - the newstat family that is used on all 64-bit architectures but

>   lacked support for large files on 32-bit architectures.

> - the stat64 family that is used mostly on 32-bit architectures to

>   replace newstat

> - statx() to replace all of the above, adding 64-bit timestamps among

>   other things.

> 

> We already compile stat64 only on those architectures that need it,

> but newstat is always built, including on those that don't reference

> it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of

> __ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of

> newstat. All architectures that need it use an explict define, the

> others now get a little bit smaller, and future architecture (including

> 64-bit targets) won't ever see it.

> 

> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Do I read this right that you only want to provide statx by default?
It is a little different from the traditional stat calls, so I'd like
to know this is actually ok from libc folks first.
Arnd Bergmann July 17, 2018, 2:18 p.m. | #2
On Tue, Jul 17, 2018 at 2:50 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 16, 2018 at 06:10:48PM +0200, Arnd Bergmann wrote:

>> We have four generations of stat() syscalls:

>> - the oldstat syscalls that are only used on the older architectures

>> - the newstat family that is used on all 64-bit architectures but

>>   lacked support for large files on 32-bit architectures.

>> - the stat64 family that is used mostly on 32-bit architectures to

>>   replace newstat

>> - statx() to replace all of the above, adding 64-bit timestamps among

>>   other things.

>>

>> We already compile stat64 only on those architectures that need it,

>> but newstat is always built, including on those that don't reference

>> it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of

>> __ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of

>> newstat. All architectures that need it use an explict define, the

>> others now get a little bit smaller, and future architecture (including

>> 64-bit targets) won't ever see it.

>>

>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>

> Do I read this right that you only want to provide statx by default?


Yes, that is correct.

> It is a little different from the traditional stat calls, so I'd like

> to know this is actually ok from libc folks first.


That would definitely help. See below for the stat implementation
I did in my musl libc prototype based on statx(). It passes the
LTP syscall tests, but that doesn't mean all the corner cases
are correct.

       Arnd

diff --git a/src/stat/fstat.c b/src/stat/fstat.c
index ab4afc0..b4f2027 100644
--- a/src/stat/fstat.c
+++ b/src/stat/fstat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
 #include <sys/stat.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -8,6 +9,9 @@ void __procfdname(char *, unsigned);

 int fstat(int fd, struct stat *st)
 {
+#ifdef __USE_TIME_BITS64
+       return __statx(fd, "", st, AT_EMPTY_PATH | AT_STATX_SYNC_AS_STAT);
+#else
        int ret = __syscall(SYS_fstat, fd, st);
        if (ret != -EBADF || __syscall(SYS_fcntl, fd, F_GETFD) < 0)
                return __syscall_ret(ret);
@@ -19,6 +23,7 @@ int fstat(int fd, struct stat *st)
 #else
        return syscall(SYS_fstatat, AT_FDCWD, buf, st, 0);
 #endif
+#endif
 }

 LFS64(fstat);
diff --git a/src/stat/fstatat.c b/src/stat/fstatat.c
index 863d526..80add64 100644
--- a/src/stat/fstatat.c
+++ b/src/stat/fstatat.c
@@ -1,10 +1,53 @@
+#define _BSD_SOURCE
 #include <sys/stat.h>
+#include <statx.h>
+#include <fcntl.h>
+#include <errno.h>
 #include "syscall.h"
 #include "libc.h"

+#ifdef __USE_TIME_BITS64
+
+#define AT_FLAGS (AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT | AT_EMPTY_PATH)
+
+int __statx(int fd, const char *restrict path, struct stat *restrict
buf, int flag)
+{
+       struct statx stx;
+       int ret;
+
+       ret = syscall(SYS_statx, fd, path, flag, STATX_BASIC_STATS, &stx);
+
+       buf->st_dev = (dev_t)stx.stx_dev_major << 32 | stx.stx_dev_minor;
+       buf->st_ino = stx.stx_ino;
+       buf->st_mode = stx.stx_mode;
+       buf->st_nlink = stx.stx_nlink;
+       buf->st_uid = stx.stx_uid;
+       buf->st_gid = stx.stx_gid;
+       buf->st_rdev = (dev_t)stx.stx_rdev_major << 32 | stx.stx_rdev_minor;
+       buf->st_size = stx.stx_size;
+       buf->st_blksize = stx.stx_blksize;
+       buf->st_blocks = stx.stx_blocks;
+       buf->st_atim.tv_sec = stx.stx_atime.tv_sec;
+       buf->st_mtim.tv_sec = stx.stx_atime.tv_sec;
+       buf->st_ctim.tv_sec = stx.stx_atime.tv_sec;
+       buf->st_atim.tv_nsec = stx.stx_atime.tv_nsec;
+       buf->st_mtim.tv_nsec = stx.stx_atime.tv_nsec;
+       buf->st_ctim.tv_nsec = stx.stx_atime.tv_nsec;
+
+       return ret;
+}
+#endif
+
 int fstatat(int fd, const char *restrict path, struct stat *restrict
buf, int flag)
 {
+#ifdef __USE_TIME_BITS64
+       if (flag & ~AT_FLAGS)
+               return __syscall_ret(-EINVAL);
+
+       return __statx(fd, path, buf, flag);
+#else
        return syscall(SYS_fstatat, fd, path, buf, flag);
+#endif
 }

 LFS64(fstatat);
diff --git a/src/stat/lstat.c b/src/stat/lstat.c
index 5e8b84f..ad2bddd 100644
--- a/src/stat/lstat.c
+++ b/src/stat/lstat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
 #include <sys/stat.h>
 #include <fcntl.h>
 #include "syscall.h"
@@ -5,7 +6,9 @@

 int lstat(const char *restrict path, struct stat *restrict buf)
 {
-#ifdef SYS_lstat
+#ifdef __USE_TIME_BITS64
+       return __statx(AT_FDCWD, path, buf, AT_SYMLINK_NOFOLLOW |
AT_STATX_SYNC_AS_STAT);
+#elif defined(SYS_lstat)
        return syscall(SYS_lstat, path, buf);
 #else
        return syscall(SYS_fstatat, AT_FDCWD, path, buf, AT_SYMLINK_NOFOLLOW);
diff --git a/src/stat/stat.c b/src/stat/stat.c
index b4433a0..599903a 100644
--- a/src/stat/stat.c
+++ b/src/stat/stat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
 #include <sys/stat.h>
 #include <fcntl.h>
 #include "syscall.h"
@@ -5,7 +6,9 @@

 int stat(const char *restrict path, struct stat *restrict buf)
 {
-#ifdef SYS_stat
+#ifdef __USE_TIME_BITS64
+       return __statx(AT_FDCWD, path, buf, AT_STATX_SYNC_AS_STAT);
+#elif defined(SYS_stat)
        return syscall(SYS_stat, path, buf);
 #else
        return syscall(SYS_fstatat, AT_FDCWD, path, buf, 0);
Joseph Myers July 18, 2018, 8:15 p.m. | #3
On Tue, 17 Jul 2018, Arnd Bergmann wrote:

> That would definitely help. See below for the stat implementation

> I did in my musl libc prototype based on statx(). It passes the

> LTP syscall tests, but that doesn't mean all the corner cases

> are correct.


Well, you definitely need explicit timestamp conversions on the result of 
statx to be usable in struct stat when userspace "long" is 64-bit, for BE 
because otherwise the integer nanoseconds will be in the wrong place for 
struct timespec, and for LE if the "__reserved is held in case we need a 
yet finer resolution." might start being returned as nonzero (for integer 
attoseconds, I suppose) in future.

-- 
Joseph S. Myers
joseph@codesourcery.com
Arnd Bergmann July 19, 2018, 8:53 a.m. | #4
On Wed, Jul 18, 2018 at 10:15 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 17 Jul 2018, Arnd Bergmann wrote:

>

>> That would definitely help. See below for the stat implementation

>> I did in my musl libc prototype based on statx(). It passes the

>> LTP syscall tests, but that doesn't mean all the corner cases

>> are correct.

>

> Well, you definitely need explicit timestamp conversions on the result of

> statx to be usable in struct stat when userspace "long" is 64-bit, for BE

> because otherwise the integer nanoseconds will be in the wrong place for

> struct timespec, and for LE if the "__reserved is held in case we need a

> yet finer resolution." might start being returned as nonzero (for integer

> attoseconds, I suppose) in future.


Right. The musl implementation I sent did that by simply copying each
field we care about from the statx structure into the stat structure
individually. This is the more or less the same thing that the kernel
does to implement stat() as well.

     Arnd

Patch

diff --git a/arch/alpha/include/asm/unistd.h b/arch/alpha/include/asm/unistd.h
index d6e29a1de4cc..edc090470023 100644
--- a/arch/alpha/include/asm/unistd.h
+++ b/arch/alpha/include/asm/unistd.h
@@ -6,6 +6,7 @@ 
 
 #define NR_SYSCALLS			523
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_GETHOSTNAME
diff --git a/arch/arm/include/asm/unistd.h b/arch/arm/include/asm/unistd.h
index 076090d2dbf5..07e58010d597 100644
--- a/arch/arm/include/asm/unistd.h
+++ b/arch/arm/include/asm/unistd.h
@@ -16,6 +16,7 @@ 
 #include <uapi/asm/unistd.h>
 #include <asm/unistd-nr.h>
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_GETHOSTNAME
 #define __ARCH_WANT_SYS_PAUSE
diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h
index 5072cbd15c82..dae1584cf017 100644
--- a/arch/arm64/include/uapi/asm/unistd.h
+++ b/arch/arm64/include/uapi/asm/unistd.h
@@ -16,5 +16,6 @@ 
  */
 
 #define __ARCH_WANT_RENAMEAT
+#define __ARCH_WANT_NEW_STAT
 
 #include <asm-generic/unistd.h>
diff --git a/arch/ia64/include/asm/unistd.h b/arch/ia64/include/asm/unistd.h
index ffb705dc9c13..c5b2620c4a4c 100644
--- a/arch/ia64/include/asm/unistd.h
+++ b/arch/ia64/include/asm/unistd.h
@@ -28,6 +28,8 @@ 
 #define __IGNORE_vfork		/* clone() */
 #define __IGNORE_umount2	/* umount() */
 
+#define __ARCH_WANT_NEW_STAT
+
 #if !defined(__ASSEMBLY__) && !defined(ASSEMBLER)
 
 #include <linux/types.h>
diff --git a/arch/m68k/include/asm/unistd.h b/arch/m68k/include/asm/unistd.h
index 30d0d3fbd4ef..db22cdadc38a 100644
--- a/arch/m68k/include/asm/unistd.h
+++ b/arch/m68k/include/asm/unistd.h
@@ -7,6 +7,7 @@ 
 
 #define NR_syscalls		380
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_OLD_STAT
 #define __ARCH_WANT_STAT64
diff --git a/arch/microblaze/include/asm/unistd.h b/arch/microblaze/include/asm/unistd.h
index a62d09420a47..a28dc770c9b2 100644
--- a/arch/microblaze/include/asm/unistd.h
+++ b/arch/microblaze/include/asm/unistd.h
@@ -15,6 +15,7 @@ 
 
 /* #define __ARCH_WANT_OLD_READDIR */
 /* #define __ARCH_WANT_OLD_STAT */
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_ALARM
 #define __ARCH_WANT_SYS_GETHOSTNAME
diff --git a/arch/mips/include/asm/unistd.h b/arch/mips/include/asm/unistd.h
index 3c09450908aa..d7878b3e16d8 100644
--- a/arch/mips/include/asm/unistd.h
+++ b/arch/mips/include/asm/unistd.h
@@ -24,6 +24,7 @@ 
 
 #ifndef __ASSEMBLY__
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_SYS_ALARM
 #define __ARCH_WANT_SYS_GETHOSTNAME
diff --git a/arch/parisc/include/asm/unistd.h b/arch/parisc/include/asm/unistd.h
index 3d507d04eb4c..b36273bacca7 100644
--- a/arch/parisc/include/asm/unistd.h
+++ b/arch/parisc/include/asm/unistd.h
@@ -141,6 +141,7 @@  type name(type1 arg1, type2 arg2, type3 arg3, type4 arg4, type5 arg5)	\
     return K_INLINE_SYSCALL(name, 5, arg1, arg2, arg3, arg4, arg5);	\
 }
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_ALARM
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index c19379f0a32e..8b37e01817be 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -22,6 +22,7 @@ 
 #include <linux/compiler.h>
 #include <linux/linkage.h>
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_ALARM
diff --git a/arch/s390/include/asm/unistd.h b/arch/s390/include/asm/unistd.h
index fd79c0d35dc4..1d181373288a 100644
--- a/arch/s390/include/asm/unistd.h
+++ b/arch/s390/include/asm/unistd.h
@@ -15,6 +15,7 @@ 
 #define __IGNORE_pkey_alloc
 #define __IGNORE_pkey_free
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_SYS_ALARM
 #define __ARCH_WANT_SYS_GETHOSTNAME
diff --git a/arch/sh/include/asm/unistd.h b/arch/sh/include/asm/unistd.h
index b36200af9ce7..a845b57eac69 100644
--- a/arch/sh/include/asm/unistd.h
+++ b/arch/sh/include/asm/unistd.h
@@ -5,6 +5,7 @@ 
 #  include <asm/unistd_64.h>
 # endif
 
+# define __ARCH_WANT_NEW_STAT
 # define __ARCH_WANT_OLD_READDIR
 # define __ARCH_WANT_OLD_STAT
 # define __ARCH_WANT_STAT64
diff --git a/arch/sparc/include/asm/unistd.h b/arch/sparc/include/asm/unistd.h
index b2a6a955113e..3544244685e1 100644
--- a/arch/sparc/include/asm/unistd.h
+++ b/arch/sparc/include/asm/unistd.h
@@ -21,6 +21,7 @@ 
 #else
 #define __NR_time		231 /* Linux sparc32                               */
 #endif
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_OLD_READDIR
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_ALARM
diff --git a/arch/x86/include/asm/unistd.h b/arch/x86/include/asm/unistd.h
index 51c4eee00732..35b66bbf8028 100644
--- a/arch/x86/include/asm/unistd.h
+++ b/arch/x86/include/asm/unistd.h
@@ -31,6 +31,7 @@ 
 
 # endif
 
+# define __ARCH_WANT_NEW_STAT
 # define __ARCH_WANT_OLD_READDIR
 # define __ARCH_WANT_OLD_STAT
 # define __ARCH_WANT_SYS_ALARM
diff --git a/arch/xtensa/include/asm/unistd.h b/arch/xtensa/include/asm/unistd.h
index ed66db3bc9bb..0d532ab60b37 100644
--- a/arch/xtensa/include/asm/unistd.h
+++ b/arch/xtensa/include/asm/unistd.h
@@ -5,6 +5,7 @@ 
 #define __ARCH_WANT_SYS_CLONE
 #include <uapi/asm/unistd.h>
 
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_STAT64
 #define __ARCH_WANT_SYS_UTIME
 #define __ARCH_WANT_SYS_LLSEEK
diff --git a/fs/stat.c b/fs/stat.c
index f8e6fb2c3657..adbfcd86c81b 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -280,6 +280,8 @@  SYSCALL_DEFINE2(fstat, unsigned int, fd, struct __old_kernel_stat __user *, stat
 
 #endif /* __ARCH_WANT_OLD_STAT */
 
+#ifdef __ARCH_WANT_NEW_STAT
+
 #if BITS_PER_LONG == 32
 #  define choose_32_64(a,b) a
 #else
@@ -378,6 +380,7 @@  SYSCALL_DEFINE2(newfstat, unsigned int, fd, struct stat __user *, statbuf)
 
 	return error;
 }
+#endif
 
 static int do_readlinkat(int dfd, const char __user *pathname,
 			 char __user *buf, int bufsiz)