[v2,06/17] y2038: Change sys_utimensat() to use __kernel_timespec

Message ID 20180716161103.16239-7-arnd@arndb.de
State New, archived
Headers

Commit Message

Arnd Bergmann July 16, 2018, 4:10 p.m. UTC
  When 32-bit architectures get changed to support 64-bit time_t,
utimensat() needs to use the new __kernel_timespec structure as its
argument.

The older utime(), utimes() and futimesat() system calls don't need a
corresponding change as they are no longer used on C libraries that have
64-bit time support.

As we do for the other syscalls that have timespec arguments, we reuse
the 'compat' syscall entry points to implement the traditional four
interfaces, and only leave the new utimensat() as a native handler,
so that the same code gets used on both 32-bit and 64-bit kernels
on each syscall.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/utimes.c              | 2 +-
 include/linux/syscalls.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
  

Comments

Christoph Hellwig July 17, 2018, 12:52 p.m. UTC | #1
On Mon, Jul 16, 2018 at 06:10:52PM +0200, Arnd Bergmann wrote:
> When 32-bit architectures get changed to support 64-bit time_t,
> utimensat() needs to use the new __kernel_timespec structure as its
> argument.
> 
> The older utime(), utimes() and futimesat() system calls don't need a
> corresponding change as they are no longer used on C libraries that have
> 64-bit time support.
> 
> As we do for the other syscalls that have timespec arguments, we reuse
> the 'compat' syscall entry points to implement the traditional four
> interfaces, and only leave the new utimensat() as a native handler,
> so that the same code gets used on both 32-bit and 64-bit kernels
> on each syscall.

I wonder about the direction here:  wouldn't it be easier to just
leave th existing syscall names as-is and introduce a new utimesat64
which uses the new timespec?  We can then drop the old legacy utimesat
for new architectures added after the cutover.
  
Arnd Bergmann July 17, 2018, 2:27 p.m. UTC | #2
On Tue, Jul 17, 2018 at 2:52 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 16, 2018 at 06:10:52PM +0200, Arnd Bergmann wrote:
>> When 32-bit architectures get changed to support 64-bit time_t,
>> utimensat() needs to use the new __kernel_timespec structure as its
>> argument.
>>
>> The older utime(), utimes() and futimesat() system calls don't need a
>> corresponding change as they are no longer used on C libraries that have
>> 64-bit time support.
>>
>> As we do for the other syscalls that have timespec arguments, we reuse
>> the 'compat' syscall entry points to implement the traditional four
>> interfaces, and only leave the new utimensat() as a native handler,
>> so that the same code gets used on both 32-bit and 64-bit kernels
>> on each syscall.
>
> I wonder about the direction here:  wouldn't it be easier to just
> leave th existing syscall names as-is and introduce a new utimesat64
> which uses the new timespec?  We can then drop the old legacy utimesat
> for new architectures added after the cutover.

We have debated both approaches over several years, but for
most syscalls, we picked the approach described above, for multiple
reasons:

- For any system call that takes a time_t derived argument, we
  already have two implementations (native and compat), so adding a
  third one requires duplicating some code.

- I want to avoid adding an implementation that is not well tested
  if possible, to make it less likely to introduce security holes or
  subtle bugs that we can't fix later without breaking the ABI.
  Using the same implementation for the new 32-bit case that
  we have for the existing 64-bit case means that this code is
  much better exercised, while reusing the compat code for the
  traditional native syscall means it gets exercised by all current
  user space, which makes it more likely to catch bugs early.

- Looking at the end result, I find it more logical to have each
  of the converted syscalls implement the same binary interface
  on both 32-bit and 64-bit architectures with the same code,
  and have the old 32-bit implementation be similarly shared.
  This is even more important once we add new architectures
  that don't even provide the 32-bit time_t interfaces and just
  leave out the old entry points.

    Arnd
  

Patch

diff --git a/fs/utimes.c b/fs/utimes.c
index 69d4b6ba1bfb..ca2c02a7985c 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -166,7 +166,7 @@  long do_utimes(int dfd, const char __user *filename, struct timespec64 *times,
 }
 
 SYSCALL_DEFINE4(utimensat, int, dfd, const char __user *, filename,
-		struct timespec __user *, utimes, int, flags)
+		struct __kernel_timespec __user *, utimes, int, flags)
 {
 	struct timespec64 tstimes[2];
 
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 615cc9b374cf..45c04d75eb95 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -513,7 +513,8 @@  asmlinkage long sys_timerfd_gettime(int ufd, struct __kernel_itimerspec __user *
 
 /* fs/utimes.c */
 asmlinkage long sys_utimensat(int dfd, const char __user *filename,
-				struct timespec __user *utimes, int flags);
+				struct __kernel_timespec __user *utimes,
+				int flags);
 
 /* kernel/acct.c */
 asmlinkage long sys_acct(const char __user *name);