[PATCH v2 08/27] drm/tegra: gr2d: Miscellaneous cleanups

[Erik Faye-Lund]

On Mon, Oct 7, 2013 at 2:52 PM, Terje Bergström <tbergstrom at nvidia.com> wrote:
> On 07.10.2013 15:14, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Mon, Oct 07, 2013 at 01:34:44PM +0200, Erik Faye-Lund wrote:
>>> On Mon, Oct 7, 2013 at 10:34 AM, Thierry Reding
>>> <thierry.reding at gmail.com> wrote:
>>>> Rework the address table code for the host1x firewall. The previous
>>>> implementation allocated a bitfield but didn't check for a valid pointer
>>>> so it could potentially crash.
>>>
>>> I don't think it could crash. The bitmaps was allocated as a 256-bit
>>> field, and the register offset gets AND'ed with 0xFF before being
>>> looked up. What am I missing?
>>
>> The pointer returned from the allocation is never checked, so it could
>> theoretically be NULL and be used regardless.
>>
>> Also I'm not sure that AND'ing with 0xff is the right thing to do here.
>
> Oops, there's a check for NULL missing. If that allocation fails, probe
> should fail, so we just need to propagate the error condition. Otherwise
> we might just leave the firewall off, and let everything go in unchecked.
>
> AND 0xff is necessary, because the same registers are mirrored in
> multiple contexts. AND removes the offset coming from context, and
> leaves just the plain register offset.
>

The offsets in the commands don't have enough bits to reach over to
the next context. The contexts are repeated at multiples of 0x4000,
and 0xFFF is the largest encodable offset. So I don't really thing the
AND is needed for *that* purpose.

> If somebody gets access to the bitmap, he has access to kernel data
> structures. GR2D firewall is the least of our worries in this case.

Indeed, that's only a problem if someone is already on the other side
of the air-tight hatch.