On Tue, Jul 31, 2018 at 2:46 AM, Christian König <christian.koenig@xxxxxxx> wrote: > Am 30.07.2018 um 22:14 schrieb Alex Deucher: >> >> On Mon, Jul 30, 2018 at 5:55 AM, Michel Dänzer <michel@xxxxxxxxxxx> wrote: >>> >>> On 2018-07-24 10:53 PM, Alex Deucher wrote: >>>> >>>> On Mon, Jul 23, 2018 at 12:32 PM, Gustavo A. R. Silva >>>> <gustavo@xxxxxxxxxxxxxx> wrote: >>>>> >>>>> idx can be indirectly controlled by user-space, hence leading to a >>>>> potential exploitation of the Spectre variant 1 vulnerability. >>>>> >>>>> This issue was detected with the help of Smatch: >>>>> >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:408 amdgpu_set_pp_force_state() >>>>> warn: potential spectre issue 'data.states' >>>>> >>>>> Fix this by sanitizing idx before using it to index data.states >>>> >>>> Is this actually necessary? We already check that idx is valid a few >>>> lines before: >>>> if (ret || idx >= ARRAY_SIZE(data.states)) { >>>> count = -EINVAL; >>>> goto fail; >>>> } >>> >>> A Spectre attack would be based on idx ending up too large, but the CPU >>> speculatively executing the following code assuming idx < >>> ARRAY_SIZE(data.states), and extracting information from the incorrectly >>> speculated code via side channels. >>> >>> I'm not sure if that's actually possible in this case, but better safe >>> than sorry? >> >> Yeah, I'm not sure. I guess this can't hurt. > > > Well is idx actually controlable by userspace in an IOCTL? I guess the > answer is no. > > On the other hand the array_index_nospec() macro makes the overhead absolute > negligible. > > So I agree that we should be better safe than sorry. Ok. Applied. Thanks, Alex _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel