On 23.05.2017 14:19, Mikko Perttunen wrote: > On 23.05.2017 13:58, Dmitry Osipenko wrote: >> On 23.05.2017 13:14, Mikko Perttunen wrote: >>> Reloc shifts are implemented (see assignment of u32 reloc_addr in do_relocs), >>> and they are required for VIC job submissions. >>> >> >> The *validation* is unimplemented. Since VIC uses the shifts, we may add a >> validation for it in a way it is done for the registers / class checks, i.e. >> compare the per address register shift value. I suppose those registers are >> documented somewhere(TRM), could you please point me to them (TRM page, reg >> name)? > > Ah, indeed, sorry. I'm not sure if it's worth implementing validation for e.g. > VIC, since it would be quite a bit of work and require many per-chip and > per-unit tables or ranges in the kernel. I think it's a better solution to just > forbid everything in the firewall for VIC (and eventually other units), since on > systems with those units the normal configuration will be IOMMU anyway. > > Anyway, For VIC, everything happens using two registers (method index and method > parameter), so you would need to first detect the method index, save that, then > wait for a method parameter write and then check that against the written > method. The TRM currently has a list methods, but doesn't list their parameter - > at least most ones taking a pointer are called *_OFFSET, but not sure if all. In > any case, I don't think it's worth implementing. > Yes, in IOMMU case firewall will be only useful as a debug feature. We may check for the IOMMU presence and bypass the firewall if it's present, or we may bypass the shifts validation only. >> >>> On 23.05.2017 03:14, Dmitry Osipenko wrote: >>>> Incorrectly shifted relocation address will cause a lower memory corruption >>>> and likely a hang on a write or a read of an arbitrary data in case of IOMMU >>>> absent. As of now there is no use for the address shifting (at least on >>>> Tegra20) and adding a proper shifting / sizes validation is much more work. >>>> Let's forbid it in the firewall till a proper validation is implemented. >>>> >>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>> --- >>>> drivers/gpu/host1x/job.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c >>>> index 190353944d23..1a1568e64ba8 100644 >>>> --- a/drivers/gpu/host1x/job.c >>>> +++ b/drivers/gpu/host1x/job.c >>>> @@ -332,6 +332,10 @@ static bool check_reloc(struct host1x_reloc *reloc, >>>> struct host1x_bo *cmdbuf, >>>> if (reloc->cmdbuf.bo != cmdbuf || reloc->cmdbuf.offset != offset) >>>> return false; >>>> >>>> + /* relocation shift value validation isn't implemented yet */ >>>> + if (reloc->shift) >>>> + return false; >>>> + >>>> return true; >>>> } >>>> >>>> >> >> -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
