possible ERESTARTNOHAND leak into userspace

possible ERESTARTNOHAND leak into userspace

[Michal Hocko]

Hi,
this has already been discussed few years back with reports that select
returned with ERESTARTNOHAND in multi-threaded applications
(http://forum.soft32.com/linux/PATCH-select-fix-sys_select-leak-ERESTARTNOHAND-userspace-ftopict338572.html)

Dave has come up with a possible explanation of the race but there was
no further follow up with a conclusion.

Just for reference:
        Thread_A				Thread_B
	CPU0					CPU1
						syscall_XYZ
core_sys_select				
  ret = -ERESTARTNOHAND;
  if (signal_pending(current))
     					do_notify_resume
     					  do_signal (clear signal pending)
  return ret;
return from syscall
no pending signal
return ERESTARTNOHAND

The race window is rather small and hard to trigger but we have seen
reports where people really saw select returning ERESTARTNOHAND (on
2.6.16 based kernel - x86_64).
I am not able to reproduce that myself neither with .16 kernel nor with
the current vanilla so I am not sure whether the problem has been fixed
already. But I do not see what prevents the race with vanilla.

Is the issue real and fixable?

Thanks
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: possible ERESTARTNOHAND leak into userspace

[Oleg Nesterov]

On 12/23, Michal Hocko wrote:
>
> Hi,
> this has already been discussed few years back with reports that select
> returned with ERESTARTNOHAND in multi-threaded applications
> (http://forum.soft32.com/linux/PATCH-select-fix-sys_select-leak-ERESTARTNOHAND-userspace-ftopict338572.html)
>
> Dave has come up with a possible explanation of the race but there was
> no further follow up with a conclusion.
>
> Just for reference:
>         Thread_A				Thread_B
> 	CPU0					CPU1
> 						syscall_XYZ
> core_sys_select
>   ret = -ERESTARTNOHAND;
>   if (signal_pending(current))
>      					do_notify_resume
>      					  do_signal (clear signal pending)

"clear signal pending" can't affect Thread_A. Even if it steals
the signal sent to Thread_A.

>   return ret;
> return from syscall
> no pending signal

please see above. Only the task itself can clear its TIF_DIGPENDING.

> return ERESTARTNOHAND

do_signal() should take care and restart the syscall.

> The race window is rather small and hard to trigger but we have seen
> reports where people really saw select returning ERESTARTNOHAND (on
> 2.6.16 based kernel - x86_64).
> I am not able to reproduce that myself neither with .16 kernel nor with
> the current vanilla so I am not sure whether the problem has been fixed
> already. But I do not see what prevents the race with vanilla.

I hope the problem was already fixed, at least I do not see anything
wrong in core_sys_select().

Oleg.

Re: possible ERESTARTNOHAND leak into userspace

[Michal Hocko]

On Fri 23-12-11 16:02:27, Oleg Nesterov wrote:
> On 12/23, Michal Hocko wrote:
> >
> > Hi,
> > this has already been discussed few years back with reports that select
> > returned with ERESTARTNOHAND in multi-threaded applications
> > (http://forum.soft32.com/linux/PATCH-select-fix-sys_select-leak-ERESTARTNOHAND-userspace-ftopict338572.html)
> >
> > Dave has come up with a possible explanation of the race but there was
> > no further follow up with a conclusion.
> >
> > Just for reference:
> >         Thread_A				Thread_B
> > 	CPU0					CPU1
> > 						syscall_XYZ
> > core_sys_select
> >   ret = -ERESTARTNOHAND;
> >   if (signal_pending(current))
> >      					do_notify_resume
> >      					  do_signal (clear signal pending)
> 
> "clear signal pending" can't affect Thread_A. Even if it steals
> the signal sent to Thread_A.

Ah, I have totally missed that signal_pending is thread local.

[...]
> > The race window is rather small and hard to trigger but we have seen
> > reports where people really saw select returning ERESTARTNOHAND (on
> > 2.6.16 based kernel - x86_64).
> > I am not able to reproduce that myself neither with .16 kernel nor with
> > the current vanilla so I am not sure whether the problem has been fixed
> > already. But I do not see what prevents the race with vanilla.
> 
> I hope the problem was already fixed, at least I do not see anything
> wrong in core_sys_select().

And core_sys_select is basically same in 2.6.16 som I am still wondering
how we could leak the value.

Anyway this seems to be a .16 specific issue, sorry about the noise.

> 
> Oleg.
> 

Thanks a lot for the response.
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic