[PATCH v2] fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

[PATCH v2] fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

Currently, we're capping the values too low in the F_GETLK64 case. The
fields in that structure are 64-bit values, so we shouldn't need to do
any sort of fixup there.

Make sure we check that assumption at build time in the future however
by ensuring that the sizes we're copying will fit.

With this, we no longer need COMPAT_OFF_T_MAX either, so remove it.

Fixes: 94073ad77fff2 (fs/locks: don't mess with the address limit in compat_fcntl64)
Reported-by: Vitaly Lipatov <lav@etersoft.ru>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 arch/arm64/include/asm/compat.h   |  1 -
 arch/mips/include/asm/compat.h    |  1 -
 arch/parisc/include/asm/compat.h  |  1 -
 arch/powerpc/include/asm/compat.h |  1 -
 arch/s390/include/asm/compat.h    |  1 -
 arch/sparc/include/asm/compat.h   |  1 -
 arch/tile/include/asm/compat.h    |  1 -
 arch/x86/include/asm/compat.h     |  1 -
 fs/fcntl.c                        | 11 +++++------
 9 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index e39d487bf724..a3c7f271ad4c 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -215,7 +215,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 /*
  * A pointer passed in from user mode. This should not
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 8e2b5b556488..49691331ada4 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -200,7 +200,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 /*
  * A pointer passed in from user mode. This should not
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 07f48827afda..acf8aa07cbe0 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -195,7 +195,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 struct compat_ipc64_perm {
 	compat_key_t key;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index a035b1e5dfa7..8a2aecfe9b02 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -185,7 +185,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 /*
  * A pointer passed in from user mode. This should not
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 1b60eb3676d5..5e6a63641a5f 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -263,7 +263,6 @@ typedef struct compat_siginfo {
 #define si_overrun	_sifields._timer._overrun
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 /*
  * A pointer passed in from user mode. This should not
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 977c3f280ba1..fa38c78de0f0 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -209,7 +209,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 /*
  * A pointer passed in from user mode. This should not
diff --git a/arch/tile/include/asm/compat.h b/arch/tile/include/asm/compat.h
index c14e36f008c8..62a7b83025dd 100644
--- a/arch/tile/include/asm/compat.h
+++ b/arch/tile/include/asm/compat.h
@@ -173,7 +173,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 struct compat_ipc64_perm {
 	compat_key_t key;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index a600a6cda9ec..2cbd75dd2fd3 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -210,7 +210,6 @@ typedef struct compat_siginfo {
 } compat_siginfo_t;
 
 #define COMPAT_OFF_T_MAX	0x7fffffff
-#define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
 struct compat_ipc64_perm {
 	compat_key_t key;
diff --git a/fs/fcntl.c b/fs/fcntl.c
index e15bcc4265d0..0522e283a4f4 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -563,6 +563,9 @@ static int put_compat_flock64(const struct flock *kfl, struct compat_flock64 __u
 {
 	struct compat_flock64 fl;
 
+	BUILD_BUG_ON(sizeof(kfl->l_start) > sizeof(ufl->l_start));
+	BUILD_BUG_ON(sizeof(kfl->l_len) > sizeof(ufl->l_len));
+
 	memset(&fl, 0, sizeof(struct compat_flock64));
 	copy_flock_fields(&fl, kfl);
 	if (copy_to_user(ufl, &fl, sizeof(struct compat_flock64)))
@@ -641,12 +644,8 @@ COMPAT_SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 		if (err)
 			break;
 		err = fcntl_getlk(f.file, convert_fcntl_cmd(cmd), &flock);
-		if (err)
-			break;
-		err = fixup_compat_flock(&flock);
-		if (err)
-			return err;
-		err = put_compat_flock64(&flock, compat_ptr(arg));
+		if (!err)
+			err = put_compat_flock64(&flock, compat_ptr(arg));
 		break;
 	case F_SETLK:
 	case F_SETLKW:
-- 
2.14.3

Re: [PATCH v2] fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

[Vitaly Lipatov]

Jeff Layton писал 15.11.17 21:03:
...
> With this, we no longer need COMPAT_OFF_T_MAX either, so remove it.
COMPAT_LOFF_T_MAX

-- 
С уважением,
Виталий Липатов,
Etersoft

Re: [PATCH v2] fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

[Jeff Layton]

On Wed, 2017-11-15 at 21:31 +0300, Vitaly Lipatov wrote:
> Jeff Layton писал 15.11.17 21:03:
> ...
> > With this, we no longer need COMPAT_OFF_T_MAX either, so remove it.
> 
> COMPAT_LOFF_T_MAX
> 

Thanks, fixed in tree, but I won't resend just for that.

Are you able to test this patch and confirm that it fixes the problem
you were seeing?

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2] fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

[Christoph Hellwig]

On Wed, Nov 15, 2017 at 01:03:45PM -0500, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> Currently, we're capping the values too low in the F_GETLK64 case. The
> fields in that structure are 64-bit values, so we shouldn't need to do
> any sort of fixup there.
> 
> Make sure we check that assumption at build time in the future however
> by ensuring that the sizes we're copying will fit.
> 
> With this, we no longer need COMPAT_OFF_T_MAX either, so remove it.

Sounds like the COMPAT_LOFF_T_MAX should be a separate patch, but otherwise
this looks good.

Reviewed-by: Christoph Hellwig <hch@lst.de>

Btw, I seems like arm has another copy of the old broken compat
code in arch/arm/kernel/sys_oabi-compat.c.  I wonder if anyone with
a vested interest in arm would be willing to clean that mess up.