[PATCH v2] vfs: constify arguments to utime family of system calls

[PATCH v2] vfs: constify arguments to utime family of system calls

[Eric Biggers]

The system calls to set file times: utime(), utimes(), futimesat(), and
utimensat(), all take in pointers to a filename and time information,
neither of which is modified.  Mark the pointed-to data as 'const' to
better reflect the semantics.

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 arch/alpha/kernel/osf_sys.c |  4 ++--
 fs/compat.c                 | 13 +++++++++----
 fs/utimes.c                 | 17 +++++++++--------
 include/linux/compat.h      |  8 ++++----
 include/linux/syscalls.h    | 12 ++++++------
 include/linux/time.h        |  3 ++-
 6 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 6cc0816..20b4c85 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -952,7 +952,7 @@ struct itimerval32
 };
 
 static inline long
-get_tv32(struct timeval *o, struct timeval32 __user *i)
+get_tv32(struct timeval *o, const struct timeval32 __user *i)
 {
 	return (!access_ok(VERIFY_READ, i, sizeof(*i)) ||
 		(__get_user(o->tv_sec, &i->tv_sec) |
@@ -1065,7 +1065,7 @@ SYSCALL_DEFINE3(osf_setitimer, int, which, struct itimerval32 __user *, in,
 }
 
 SYSCALL_DEFINE2(osf_utimes, const char __user *, filename,
-		struct timeval32 __user *, tvs)
+		const struct timeval32 __user *, tvs)
 {
 	struct timespec tv[2];
 
diff --git a/fs/compat.c b/fs/compat.c
index a71936a..5a6aa35 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -73,7 +73,7 @@ int compat_printk(const char *fmt, ...)
  * of sys_utimes.
  */
 COMPAT_SYSCALL_DEFINE2(utime, const char __user *, filename,
-		       struct compat_utimbuf __user *, t)
+		       const struct compat_utimbuf __user *, t)
 {
 	struct timespec tv[2];
 
@@ -87,7 +87,9 @@ COMPAT_SYSCALL_DEFINE2(utime, const char __user *, filename,
 	return do_utimes(AT_FDCWD, filename, t ? tv : NULL, 0);
 }
 
-COMPAT_SYSCALL_DEFINE4(utimensat, unsigned int, dfd, const char __user *, filename, struct compat_timespec __user *, t, int, flags)
+COMPAT_SYSCALL_DEFINE4(utimensat, unsigned int, dfd,
+		       const char __user *, filename,
+		       const struct compat_timespec __user *, t, int, flags)
 {
 	struct timespec tv[2];
 
@@ -102,7 +104,9 @@ COMPAT_SYSCALL_DEFINE4(utimensat, unsigned int, dfd, const char __user *, filena
 	return do_utimes(dfd, filename, t ? tv : NULL, flags);
 }
 
-COMPAT_SYSCALL_DEFINE3(futimesat, unsigned int, dfd, const char __user *, filename, struct compat_timeval __user *, t)
+COMPAT_SYSCALL_DEFINE3(futimesat, unsigned int, dfd,
+		       const char __user *, filename,
+		       const struct compat_timeval __user *, t)
 {
 	struct timespec tv[2];
 
@@ -121,7 +125,8 @@ COMPAT_SYSCALL_DEFINE3(futimesat, unsigned int, dfd, const char __user *, filena
 	return do_utimes(dfd, filename, t ? tv : NULL, 0);
 }
 
-COMPAT_SYSCALL_DEFINE2(utimes, const char __user *, filename, struct compat_timeval __user *, t)
+COMPAT_SYSCALL_DEFINE2(utimes, const char __user *, filename,
+		       const struct compat_timeval __user *, t)
 {
 	return compat_sys_futimesat(AT_FDCWD, filename, t);
 }
diff --git a/fs/utimes.c b/fs/utimes.c
index 85c40f4..4954214 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -24,7 +24,8 @@
  * must be owner or have write permission.
  * Else, update from *times, must be owner or super user.
  */
-SYSCALL_DEFINE2(utime, char __user *, filename, struct utimbuf __user *, times)
+SYSCALL_DEFINE2(utime, const char __user *, filename,
+		const struct utimbuf __user *, times)
 {
 	struct timespec tv[2];
 
@@ -48,7 +49,7 @@ static bool nsec_valid(long nsec)
 	return nsec >= 0 && nsec <= 999999999;
 }
 
-static int utimes_common(struct path *path, struct timespec *times)
+static int utimes_common(struct path *path, const struct timespec *times)
 {
 	int error;
 	struct iattr newattrs;
@@ -133,8 +134,8 @@ out:
  * must be owner or have write permission.
  * Else, update from *times, must be owner or super user.
  */
-long do_utimes(int dfd, const char __user *filename, struct timespec *times,
-	       int flags)
+long do_utimes(int dfd, const char __user *filename,
+	       const struct timespec *times, int flags)
 {
 	int error = -EINVAL;
 
@@ -183,7 +184,7 @@ out:
 }
 
 SYSCALL_DEFINE4(utimensat, int, dfd, const char __user *, filename,
-		struct timespec __user *, utimes, int, flags)
+		const struct timespec __user *, utimes, int, flags)
 {
 	struct timespec tstimes[2];
 
@@ -201,7 +202,7 @@ SYSCALL_DEFINE4(utimensat, int, dfd, const char __user *, filename,
 }
 
 SYSCALL_DEFINE3(futimesat, int, dfd, const char __user *, filename,
-		struct timeval __user *, utimes)
+		const struct timeval __user *, utimes)
 {
 	struct timeval times[2];
 	struct timespec tstimes[2];
@@ -228,8 +229,8 @@ SYSCALL_DEFINE3(futimesat, int, dfd, const char __user *, filename,
 	return do_utimes(dfd, filename, utimes ? tstimes : NULL, 0);
 }
 
-SYSCALL_DEFINE2(utimes, char __user *, filename,
-		struct timeval __user *, utimes)
+SYSCALL_DEFINE2(utimes, const char __user *, filename,
+		const struct timeval __user *, utimes)
 {
 	return sys_futimesat(AT_FDCWD, filename, utimes);
 }
diff --git a/include/linux/compat.h b/include/linux/compat.h
index f964ef7..def595d 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -461,10 +461,10 @@ asmlinkage long compat_sys_epoll_pwait(int epfd,
 			compat_size_t sigsetsize);
 
 asmlinkage long compat_sys_utime(const char __user *filename,
-				 struct compat_utimbuf __user *t);
+				 const struct compat_utimbuf __user *t);
 asmlinkage long compat_sys_utimensat(unsigned int dfd,
 				     const char __user *filename,
-				     struct compat_timespec __user *t,
+				     const struct compat_timespec __user *t,
 				     int flags);
 
 asmlinkage long compat_sys_time(compat_time_t __user *tloc);
@@ -485,9 +485,9 @@ asmlinkage long compat_sys_move_pages(pid_t pid, compat_ulong_t nr_pages,
 				      int flags);
 asmlinkage long compat_sys_futimesat(unsigned int dfd,
 				     const char __user *filename,
-				     struct compat_timeval __user *t);
+				     const struct compat_timeval __user *t);
 asmlinkage long compat_sys_utimes(const char __user *filename,
-				  struct compat_timeval __user *t);
+				  const struct compat_timeval __user *t);
 asmlinkage long compat_sys_newstat(const char __user *filename,
 				   struct compat_stat __user *statbuf);
 asmlinkage long compat_sys_newlstat(const char __user *filename,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index d795472..3b83e59 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -550,10 +550,10 @@ asmlinkage long sys_getgid16(void);
 asmlinkage long sys_getegid16(void);
 #endif
 
-asmlinkage long sys_utime(char __user *filename,
-				struct utimbuf __user *times);
-asmlinkage long sys_utimes(char __user *filename,
-				struct timeval __user *utimes);
+asmlinkage long sys_utime(const char __user *filename,
+			  const struct utimbuf __user *times);
+asmlinkage long sys_utimes(const char __user *filename,
+			   const struct timeval __user *utimes);
 asmlinkage long sys_lseek(unsigned int fd, off_t offset,
 			  unsigned int whence);
 asmlinkage long sys_llseek(unsigned int fd, unsigned long offset_high,
@@ -771,7 +771,7 @@ asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
 			      int newdfd, const char __user *newname,
 			      unsigned int flags);
 asmlinkage long sys_futimesat(int dfd, const char __user *filename,
-			      struct timeval __user *utimes);
+			      const struct timeval __user *utimes);
 asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode);
 asmlinkage long sys_fchmodat(int dfd, const char __user * filename,
 			     umode_t mode);
@@ -784,7 +784,7 @@ asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
 asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *buf,
 			       int bufsiz);
 asmlinkage long sys_utimensat(int dfd, const char __user *filename,
-				struct timespec __user *utimes, int flags);
+			      const struct timespec __user *utimes, int flags);
 asmlinkage long sys_unshare(unsigned long unshare_flags);
 
 asmlinkage long sys_splice(int fd_in, loff_t __user *off_in,
diff --git a/include/linux/time.h b/include/linux/time.h
index 297f09f..20b28fb 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -174,7 +174,8 @@ extern int do_getitimer(int which, struct itimerval *value);
 
 extern unsigned int alarm_setitimer(unsigned int seconds);
 
-extern long do_utimes(int dfd, const char __user *filename, struct timespec *times, int flags);
+extern long do_utimes(int dfd, const char __user *filename,
+		      const struct timespec *times, int flags);
 
 struct tms;
 extern void do_sys_times(struct tms *);
-- 
2.7.4

Re: [PATCH v2] vfs: constify arguments to utime family of system calls

[Al Viro]

On Thu, Mar 31, 2016 at 08:51:12PM -0500, Eric Biggers wrote:
> The system calls to set file times: utime(), utimes(), futimesat(), and
> utimensat(), all take in pointers to a filename and time information,
> neither of which is modified.  Mark the pointed-to data as 'const' to
> better reflect the semantics.

I'm not sure if there's any point, to be honest.  Anything doing direct
dereferencing of those pointers is completely broken and reads are no
better than writes in that respect.  OTOH... put_user() and friends will
produce an error when you give them const void __user * on most of the
architectures, so it's not entirely useles...

What's the situation with other syscalls?  It doesn't make much sense
to do it piece-by-piece...

Re: [PATCH v2] vfs: constify arguments to utime family of system calls

[Eric Biggers]

On Fri, Apr 01, 2016 at 03:05:24AM +0100, Al Viro wrote:
> On Thu, Mar 31, 2016 at 08:51:12PM -0500, Eric Biggers wrote:
> > The system calls to set file times: utime(), utimes(), futimesat(), and
> > utimensat(), all take in pointers to a filename and time information,
> > neither of which is modified.  Mark the pointed-to data as 'const' to
> > better reflect the semantics.
> 
> I'm not sure if there's any point, to be honest.  Anything doing direct
> dereferencing of those pointers is completely broken and reads are no
> better than writes in that respect.  OTOH... put_user() and friends will
> produce an error when you give them const void __user * on most of the
> architectures, so it's not entirely useles...
> 
> What's the situation with other syscalls?  It doesn't make much sense
> to do it piece-by-piece...

I feel it's a small improvement as it reduces the chance of bugs.  However, if
you look at all the system calls, they are, in general, inconsistent about using
'const'.  So may be right that changing just a few isn't worthwhile.

Re: [PATCH v2] vfs: constify arguments to utime family of system calls

[Al Viro]

On Thu, Mar 31, 2016 at 09:24:57PM -0500, Eric Biggers wrote:

> I feel it's a small improvement as it reduces the chance of bugs.  However, if
> you look at all the system calls, they are, in general, inconsistent about using
> 'const'.  So may be right that changing just a few isn't worthwhile.

It might very well be worth doing a full series, though - after all,
they are not hard to grep for ((COMPAT_)?SYSCALL_DEFINE[0-6] would get
you almost all of them, with rest consisting of weird shit in arch/*
that more often than not would be better off switched to SYSCALL_DEFINE).

The reason why it's worth doing completely or not at all is that partial
series will serve as an invitation to the alt.sex.masturbation.commit.count
crowd, with the usual quality of output...