svn commit: r303503 - head/sys/kern

Bruce Evans brde at optusnet.com.au
Sat Jul 30 00:38:44 UTC 2016


On Fri, 29 Jul 2016, John Baldwin wrote:

> Log:
>  Don't treat NOCPU as a valid CPU to CPU_ISSET.
>
>  If a thread is created bound to a cpuset it might already be bound before
>  it's very first timeslice, and td_lastcpu will be NOCPU in that case.
>
>  MFC after:	1 week

Thanks.  Did you get this from your mail queue on 2016/05/08?
>
> Modified: head/sys/kern/sched_4bsd.c
> ==============================================================================
> --- head/sys/kern/sched_4bsd.c	Fri Jul 29 19:36:10 2016	(r303502)
> +++ head/sys/kern/sched_4bsd.c	Fri Jul 29 20:19:14 2016	(r303503)
> @@ -1241,7 +1241,7 @@ sched_pickcpu(struct thread *td)
>
> 	mtx_assert(&sched_lock, MA_OWNED);
>
> -	if (THREAD_CAN_SCHED(td, td->td_lastcpu))
> +	if (td->td_lastcpu != NOCPU && THREAD_CAN_SCHED(td, td->td_lastcpu))
> 		best = td->td_lastcpu;
> 	else
> 		best = NOCPU;

This bug was more fatal on amd64 than on i386.  td_last_cpu has a correct
(signed) type int and NOCPU is -1.  THREAD_CAN_SCHED uses unsigned type.
-1 becomes 0xFFFFFFFFFFFFFFFF in it on amd64 and 0xFFFFFFFF on i386.
The old mail says that these get shifted to 1/4 as big, but I think
the relevant value is more like 8 -- 8 bits per byte requires a byte
at offset 1/8 of of these values in a byte array for a bitmap, but the
array is of u_int or u_long so the array index is 1/32nd or 1/64 of
these values and the memory offset is 1/8.  Anyway, the final offset
is small enough to not always trap on i386 only.

When NOCPU was 255, t_lastcpu was u_char and the memory offset was 31
bytes.  This didn't even give a buffer overrun with MAXCPU = 254.

When td_lastcpu is NOCPU and the check doesn't trap, 'best' is set to
NOCPU in both cases and the code works.

Bruce


More information about the svn-src-all mailing list