Skip to contentSkip to conversation

fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

PATCH Superseded Jeff Layton 2 participants 2 messages Nov 15, 2017

Subsystems (2)
VFS File locking (flock() and fcntl()/lockf())

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jeff Layton

author maintainer
#0 lore raw

PATCH 1/1 fcntl: don't cap l_start and l_end values for F_GETLK64 in compat syscall

From: Jeff Layton <[email protected]>

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 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.

Fixes 94073ad77fff2 (fs/locks: don't mess with the address limit in compat_fcntl64)
Reported-by Vitaly Lipatov <[email protected]>
Signed-off-by Jeff Layton <[email protected]>
---
 fs/fcntl.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
fs/fcntl.c +5 -6
index e15bcc4265d0..0522e283a4f4 100644
@@ -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:
Signature · 1 line
2.14.3
#1 lore raw

Jeff Layton писал 15.11.17 16:17:

Quoted 8 lines

From: Jeff Layton <[email protected]>

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 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.

Nice fix.
The only question I do not know answer is what is purpose of COMPAT_LOFF_T_MAX macro?

Signature · 3 lines
С уважением,
Виталий Липатов,
Etersoft