| Message ID | 20161209112131.3924-10-christian.gmeiner@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner: > Reading some registers end in a system crash ala: > > Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000 > Internal error: : 1028 [#1] PREEMPT ARM > > Avoid those by register validation. Avoiding crashes is one thing, but I believe this needs to go further and avoid reads from any register that isn't a performance counter. This probably isn't a big hole, but we want to avoid leaking the GPU state to userspace. Regards, Lucas > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > index 08f9b3d..4383c0d 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > @@ -277,6 +277,27 @@ static int submit_reloc(struct etnaviv_gem_submit *submit, void *stream, > return 0; > } > > +static int readback_reg_valid(unsigned reg) > +{ > + /* > + * 0x000..0x200: ok > + * 0x200..0x400: crash > + * 0x400..0x800: ok > + * 0x800..0xa00: crash > + * 0xa00..0xc00: crash > + * 0xc00..0xe00: crash > + * 0xe00..0x1000: crash > + * everything above: crash > + */ > + if (reg >= 0x200 && reg < 400) > + return 0; > + > + if (reg >= 0x800) > + return 0; > + > + return 1; > +} > + > static int submit_readback(struct etnaviv_gem_submit *submit, > struct etnaviv_cmdbuf *cmdbuf, > const struct drm_etnaviv_gem_submit_readback *readbacks, > @@ -303,6 +324,11 @@ static int submit_readback(struct etnaviv_gem_submit *submit, > return -EINVAL; > } > > + if (!readback_reg_valid(r->reg)) { > + DRM_ERROR("invalid readback reg (would cause crash)"); > + return -EINVAL; > + } > + > cmdbuf->readbacks[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base); > cmdbuf->readbacks[i].offset = r->readback_offset; > cmdbuf->readbacks[i].reg = r->reg;
On Mon, Jan 30, 2017 at 11:58:32AM +0100, Lucas Stach wrote: > Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner: > > Reading some registers end in a system crash ala: > > > > Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000 > > Internal error: : 1028 [#1] PREEMPT ARM > > > > Avoid those by register validation. > > Avoiding crashes is one thing, but I believe this needs to go further > and avoid reads from any register that isn't a performance counter. This > probably isn't a big hole, but we want to avoid leaking the GPU state to > userspace. We certainly don't want to let people use this to read stuff like the ID registers, bypassing the kernel.
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 08f9b3d..4383c0d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -277,6 +277,27 @@ static int submit_reloc(struct etnaviv_gem_submit *submit, void *stream, return 0; } +static int readback_reg_valid(unsigned reg) +{ + /* + * 0x000..0x200: ok + * 0x200..0x400: crash + * 0x400..0x800: ok + * 0x800..0xa00: crash + * 0xa00..0xc00: crash + * 0xc00..0xe00: crash + * 0xe00..0x1000: crash + * everything above: crash + */ + if (reg >= 0x200 && reg < 400) + return 0; + + if (reg >= 0x800) + return 0; + + return 1; +} + static int submit_readback(struct etnaviv_gem_submit *submit, struct etnaviv_cmdbuf *cmdbuf, const struct drm_etnaviv_gem_submit_readback *readbacks, @@ -303,6 +324,11 @@ static int submit_readback(struct etnaviv_gem_submit *submit, return -EINVAL; } + if (!readback_reg_valid(r->reg)) { + DRM_ERROR("invalid readback reg (would cause crash)"); + return -EINVAL; + } + cmdbuf->readbacks[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base); cmdbuf->readbacks[i].offset = r->readback_offset; cmdbuf->readbacks[i].reg = r->reg;
Reading some registers end in a system crash ala: Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000 Internal error: : 1028 [#1] PREEMPT ARM Avoid those by register validation. Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)