Skip to contentSkip to conversation

compat: poll() in 32-bit applications does not handle negative timeout values properly on 64-bit kernels

RFC PATCH In review Josh Hunt 2 participants 3 messages Feb 11, 2012 – Feb 16, 2012

Subsystems (3)
x86 VFS Generic include/asm header files
lore
Share & tools
lore

Josh Hunt

author
#0 lore raw

RFC PATCH v2 1/1 compat: poll() in 32-bit applications does not handle negative timeout values properly on 64-bit kernels

We have hit an issue where our 32-bit applications using poll() and passing in a value of -1 for the timeout value return after ~49 days (2^32 msec), instead of waiting indefinitely. I've instrumented the kernel and found we are hitting the case where poll() believes we've passed in a positive number and thus creates a timespec, etc. I've implemented compat_sys_poll() to sign-extend the timeout value and resolve the issue.

There was an almost identical patch submitted last year, but for whatever reason did not make it in:
https://lkml.org/lkml/2011/9/18/19

I am guessing there are other architectures affected by this bug. This patch only fixes x86.

Josh

commit cde9eb901ccb3b5af3e501b018b90f16c53942c2

Author: Josh Hunt <[email protected]>
Date:   Mon Feb 6 20:51:31 2012 -0800

compat: poll() in 32-bit applications does not handle negative timeout values properly on 64-bit kernels

We have observed our 32-bit applications running on 64-bit kernels do not wait infinitely when passed a negative value for the timeout argument. Instead we see poll() returning in ~49 days or 2^32 msecs, because the timeout argument is not getting sign-extended. Implementing compat_sys_poll() to handle this case.

Reported-by Phil Lisiecki <[email protected]>
Signed-off-by Josh Hunt <[email protected]>
Cc <[email protected]>
arch/x86/syscalls/syscall_32.tbl +1 -1
index ce98e28..8407150 100644
@@ -174,7 +174,7 @@
 165	i386	getresuid		sys_getresuid16
 166	i386	vm86			ptregs_vm86			sys32_vm86_warning
 167	i386	query_module
-168	i386	poll			sys_poll
+168	i386	poll			sys_poll			compat_sys_poll
 169	i386	nfsservctl
 170	i386	setresgid		sys_setresgid16
 171	i386	getresgid		sys_getresgid16
fs/compat.c +6 -0
index fa9d721..77bd50e 100644
@@ -1611,6 +1611,12 @@ asmlinkage long compat_sys_pselect6(int n, compat_ulong_t __user *inp,
 				 sigsetsize);
 }
 
+asmlinkage long compat_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
+	int timeout_msecs)
+{
+	return sys_poll(ufds, nfds, timeout_msecs);
+}
+
 asmlinkage long compat_sys_ppoll(struct pollfd __user *ufds,
 	unsigned int nfds, struct compat_timespec __user *tsp,
 	const compat_sigset_t __user *sigmask, compat_size_t sigsetsize)
include/linux/compat.h +2 -0
index 41c9f65..66e61e0 100644
@@ -433,6 +433,8 @@ asmlinkage long compat_sys_pselect6(int n, compat_ulong_t __user *inp,
 				    compat_ulong_t __user *exp,
 				    struct compat_timespec __user *tsp,
 				    void __user *sig);
+asmlinkage long compat_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
+				int timeout_msecs);
 asmlinkage long compat_sys_ppoll(struct pollfd __user *ufds,
 				 unsigned int nfds,
 				 struct compat_timespec __user *tsp,
Acked-by on v2 1/1
#1 lore raw
Quoted 11 lines · Josh Hunt

We have hit an issue where our 32-bit applications using poll() and passing in a value of -1 for the timeout value return after ~49 days (2^32 msec), instead of waiting indefinitely. I've instrumented the kernel and found we are hitting the case where poll() believes we've passed in a positive number and thus creates a timespec, etc. I've implemented compat_sys_poll() to sign-extend the timeout value and resolve the issue.

There was an almost identical patch submitted last year, but for whatever reason did not make it in:
https://lkml.org/lkml/2011/9/18/19

Probably because Thomas Meyer didnt answer to patch reviewers, and not enough people cared.

By the way, an inline patch is better, as stated in Documentation/SubmittingPatches

Anyway, its time to fix this bug...

Acked-by Eric Dumazet <[email protected]>
author
#2 lore raw
Quoted 16 lines · Eric Dumazet
Quoted 11 lines · Josh Hunt

We have hit an issue where our 32-bit applications using poll() and passing in a value of -1 for the timeout value return after ~49 days (2^32 msec), instead of waiting indefinitely. I've instrumented the kernel and found we are hitting the case where poll() believes we've passed in a positive number and thus creates a timespec, etc. I've implemented compat_sys_poll() to sign-extend the timeout value and resolve the issue.

There was an almost identical patch submitted last year, but for whatever reason did not make it in:
https://lkml.org/lkml/2011/9/18/19

Probably because Thomas Meyer didnt answer to patch reviewers, and not enough people cared.

Quoted 7 lines

By the way, an inline patch is better, as stated in Documentation/SubmittingPatches

Anyway, its time to fix this bug...

Acked-by Eric Dumazet <[email protected]>

I can resubmit the patch inline if you feel that will get more attention by whoever needs to do the final approval on this.

Josh