[PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace

[PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace

[Neil Horman]

As it is currently written, sys_select checks its return code to convert
ERESTARTNOHAND to EINTR.  However, the check is within an if (tvp) clause, and
so if select is called from userspace with a NULL timeval, then it is possible
for the ERESTARTNOHAND errno to leak into userspace, which is incorrect.  This
patch moves that check outside of the conditional, and prevents the errno leak.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 select.c |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)


diff --git a/fs/select.c b/fs/select.c
index fe0893a..afcd691 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -415,20 +415,12 @@ asmlinkage long sys_select(int n, fd_set __user *inp, fd_set __user *outp,
 		rtv.tv_sec = timeout;
 		if (timeval_compare(&rtv, &tv) >= 0)
 			rtv = tv;
-		if (copy_to_user(tvp, &rtv, sizeof(rtv))) {
-sticky:
-			/*
-			 * If an application puts its timeval in read-only
-			 * memory, we don't want the Linux-specific update to
-			 * the timeval to cause a fault after the select has
-			 * completed successfully. However, because we're not
-			 * updating the timeval, we can't restart the system
-			 * call.
-			 */
-			if (ret == -ERESTARTNOHAND)
-				ret = -EINTR;
-		}
+		if (copy_to_user(tvp, &rtv, sizeof(rtv)))
+			return -EFAULT;
 	}
+sticky:
+	if (ret == -ERESTARTNOHAND)
+		ret = -EINTR;
 
 	return ret;
 }

Re: [PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace

[Andi Kleen]

On Tuesday 23 January 2007 00:00, Neil Horman wrote:
> As it is currently written, sys_select checks its return code to convert
> ERESTARTNOHAND to EINTR.  However, the check is within an if (tvp) clause,
> and so if select is called from userspace with a NULL timeval, then it is
> possible for the ERESTARTNOHAND errno to leak into userspace, which is
> incorrect.  This patch moves that check outside of the conditional, and
> prevents the errno leak.


Patch looks good to me.

-Andi

Re: [PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace

[Neil Horman]

On Tue, Jan 23, 2007 at 10:02:49AM +1100, Andi Kleen wrote:
> On Tuesday 23 January 2007 00:00, Neil Horman wrote:
> > As it is currently written, sys_select checks its return code to convert
> > ERESTARTNOHAND to EINTR.  However, the check is within an if (tvp) clause,
> > and so if select is called from userspace with a NULL timeval, then it is
> > possible for the ERESTARTNOHAND errno to leak into userspace, which is
> > incorrect.  This patch moves that check outside of the conditional, and
> > prevents the errno leak.
> 
> 
> Patch looks good to me.
> 
> -Andi
Thanks for the response, but there is some question as to this patchs validity.
I'm waiting on a reproducer to try and veryify the problem.

Neil