Re: [PATCH 12/18] Thermal/int340x: prevent bounds-check bypass via speculative execution

From: Dan Williams
Date: Fri Jan 05 2018 - 20:57:24 EST


On Fri, Jan 5, 2018 at 5:53 PM, Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> On Fri, 2018-01-05 at 17:10 -0800, Dan Williams wrote:
>> Static analysis reports that 'trip' may be a user controlled value
>> that
>> is used as a data dependency to read '*temp' from the 'd->aux_trips'
>> array. In order to avoid potential leaks of kernel memory values,
>> block
>> speculative execution of the instruction stream that could issue
>> reads
>> based on an invalid value of '*temp'.
>
> Not against the change as this is in a very slow path. But the trip is
> not an arbitrary value which user can enter.
>
> This trip value is the one of the sysfs attribute in thermal zone. For
> example
>
> # cd /sys/class/thermal/thermal_zone1
> # ls trip_point_?_temp
> trip_point_0_temp trip_point_1_temp trip_point_2_temp trip_point_3_t
> emp trip_point_4_temp trip_point_5_temp trip_point_6_temp
>
> Here the "trip" is one of the above trip_point_*_temp. So in this case
> it can be from 0 to 6 as user can't do
> # cat trip_point_7_temp
> as there is no sysfs attribute for trip_point_7_temp.
>
> The actual "trip" was obtained in thermal core via
>
> if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip) != 1)
> return -EINVAL;
>
> Thanks,
> Srinivas

Ah, great, thanks. So do we even need the bounds check at that point?