Re: [Mesa-dev] [PATCH] etnaviv: fix refcnt initialization in etna_screen

[Christian Gmeiner]

2017-07-06 23:18 GMT+02:00 Aleksander Morgado :
> Despite being a member of the etna_screen struct, 'refcnt' is used by
> the winsys-specific logic to track the reference count of the object
> managed in a hash table. When the count reaches zero, the pipe screen
> is removed from the table and destroyed.
>
> Fix the logic by initializing the refcnt to 1 when screen created.
> This initialization is done in etna_screen_create(), to follow the
> same logic as in freedreno and virgl.
>
> Signed-off-by: Aleksander Morgado 

Reviewed-by: Christian Gmeiner 

> ---
>  src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index eefb51c5da..fa0cbd9076 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -772,6 +772,7 @@ etna_screen_create(struct etna_device *dev, struct 
> etna_gpu *gpu,
> screen->dev = dev;
> screen->gpu = gpu;
> screen->ro = renderonly_dup(ro);
> +   screen->refcnt = 1;
>
> if (!screen->ro) {
>DBG("could not create renderonly object");
> --
> 2.13.1
>

greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] etnaviv: fix refcnt initialization in etna_screen

[Emil Velikov]

On 7 July 2017 at 09:09, Lucas Stach  wrote:
> Am Donnerstag, den 06.07.2017, 23:18 +0200 schrieb Aleksander Morgado:
>> Despite being a member of the etna_screen struct, 'refcnt' is used by
>> the winsys-specific logic to track the reference count of the object
>> managed in a hash table. When the count reaches zero, the pipe screen
>> is removed from the table and destroyed.
>>
>> Fix the logic by initializing the refcnt to 1 when screen created.
>> This initialization is done in etna_screen_create(), to follow the
>> same logic as in freedreno and virgl.
>
> Urgh, nice (for whatever value of nice) find. I'll push this with stable
> tags added in a moment.
>
It should land in stable indeed. Thanks Lucas.

Very quick skim shows that other drivers don't have this bug, yet I
wonder if we cannot get a volunteer to re-spin RobH refcount
series[1].
There were some comments but nothing serious IMHO.

Also, welcome to Mesa Aleksander!
Emil

[1] https://patchwork.freedesktop.org/series/8855/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] etnaviv: fix refcnt initialization in etna_screen

[Lucas Stach]

Am Donnerstag, den 06.07.2017, 23:18 +0200 schrieb Aleksander Morgado:
> Despite being a member of the etna_screen struct, 'refcnt' is used by
> the winsys-specific logic to track the reference count of the object
> managed in a hash table. When the count reaches zero, the pipe screen
> is removed from the table and destroyed.
> 
> Fix the logic by initializing the refcnt to 1 when screen created.
> This initialization is done in etna_screen_create(), to follow the
> same logic as in freedreno and virgl.

Urgh, nice (for whatever value of nice) find. I'll push this with stable
tags added in a moment.

Regards,
Lucas

> Signed-off-by: Aleksander Morgado 
> ---
>  src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index eefb51c5da..fa0cbd9076 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -772,6 +772,7 @@ etna_screen_create(struct etna_device *dev, struct 
> etna_gpu *gpu,
> screen->dev = dev;
> screen->gpu = gpu;
> screen->ro = renderonly_dup(ro);
> +   screen->refcnt = 1;
>  
> if (!screen->ro) {
>DBG("could not create renderonly object");


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] etnaviv: fix refcnt initialization in etna_screen

[Aleksander Morgado]

On Thu, Jul 6, 2017 at 11:18 PM, Aleksander Morgado
 wrote:
> Despite being a member of the etna_screen struct, 'refcnt' is used by
> the winsys-specific logic to track the reference count of the object
> managed in a hash table. When the count reaches zero, the pipe screen
> is removed from the table and destroyed.
>
> Fix the logic by initializing the refcnt to 1 when screen created.
> This initialization is done in etna_screen_create(), to follow the
> same logic as in freedreno and virgl.
>

For reference, this is the kind of backtrace I was getting due to this
issue. The dri2_create_image_from_winsys() call was trying run
pscreen->resource_from_handle, but the pscreen had already been freed.

If the item is added to the HT with refcnt = 0, getting an extra
reference from the HT would have set rfcnt = 1, and when that extra
reference was removed it would have gone to rfcnt = 0, triggering at
this point the destroy and removal from the HT, while the original
reference was still around and assumed valid, and finally arriving to
the use-after-free seen here.

01-01 00:00:51.546   621   621 F libc: Fatal signal 11 (SIGSEGV),
code 1, fault addr 0x0 in tid 621 (ndroid.systemui)
01-01 00:00:51.679  1062  1062 F DEBUG   : *** *** *** *** *** *** ***
*** *** *** *** *** *** *** *** ***
01-01 00:00:51.679  1062  1062 F DEBUG   : Build fingerprint:
'Android/linaro_arm/linaro_arm:7.1.1/N6F26U/aleksa04281944:userdebug/test-keys'
01-01 00:00:51.679  1062  1062 F DEBUG   : Revision: '0'
01-01 00:00:51.679  1062  1062 F DEBUG   : ABI: 'arm'
01-01 00:00:51.680  1062  1062 F DEBUG   : pid: 621, tid: 621, name:
ndroid.systemui  >>> com.android.systemui <<<
01-01 00:00:51.680  1062  1062 F DEBUG   : signal 11 (SIGSEGV), code 1
(SEGV_MAPERR), fault addr 0x0
01-01 00:00:51.680  1062  1062 F DEBUG   : r0 91e65d00  r1
bec0d3d8  r2 bec0d470  r3 0006
01-01 00:00:51.681  1062  1062 F DEBUG   : r4 bec0d470  r5
91e65d00  r6   r7 bec0d3ea
01-01 00:00:51.681  1062  1062 F DEBUG   : r8 914deb27  r9
  sl 8ef239c0  fp 1005
01-01 00:00:51.681  1062  1062 F DEBUG   : ip bec0cef4  sp
bec0d3c0  lr 91242bd9  pc   cpsr 2010
01-01 00:00:52.711  1062  1062 F DEBUG   :
01-01 00:00:52.711  1062  1062 F DEBUG   : backtrace:
01-01 00:00:52.711  1062  1062 F DEBUG   : #00 pc   
01-01 00:00:52.711  1062  1062 F DEBUG   : #01 pc 00066bd7
/system/lib/dri/gallium_dri.so (dri2_create_image_from_winsys+462)
01-01 00:00:52.711  1062  1062 F DEBUG   : #02 pc 00066f49
/system/lib/dri/gallium_dri.so (dri2_create_image_from_fd+592)
01-01 00:00:52.712  1062  1062 F DEBUG   : #03 pc 00065e07
/system/lib/dri/gallium_dri.so (dri2_from_dma_bufs2+54)
01-01 00:00:52.712  1062  1062 F DEBUG   : #04 pc 1fb7
/system/lib/libgbm.so (gbm_dri_bo_import+418)
01-01 00:00:52.712  1062  1062 F DEBUG   : #05 pc 127f
/system/lib/hw/gralloc.gbm.so
(_ZL15validate_handlePK13native_handleP10gbm_device+334)
01-01 00:00:52.712  1062  1062 F DEBUG   : #06 pc 111f
/system/lib/hw/gralloc.gbm.so (gralloc_gbm_handle_register+2)
01-01 00:00:52.712  1062  1062 F DEBUG   : #07 pc 160f
/system/lib/hw/gralloc.gbm.so
(_ZL23gbm_mod_register_bufferPK16gralloc_module_tPK13native_handle+28)
01-01 00:00:52.712  1062  1062 F DEBUG   : #08 pc cbdd
/system/lib/libui.so
(_ZN7android18Gralloc1On0Adapter6retainEPKNS_13GraphicBufferE+80)
01-01 00:00:52.712  1062  1062 F DEBUG   : #09 pc f45b
/system/lib/libui.so
(_ZN7android19GraphicBufferMapper14registerBufferEPKNS_13GraphicBufferE+58)
01-01 00:00:52.713  1062  1062 F DEBUG   : #10 pc e7c5
/system/lib/libui.so
(_ZN7android13GraphicBuffer9unflattenERPKvRjRPKiS4_+300)
01-01 00:00:52.713  1062  1062 F DEBUG   : #11 pc 00083951
/system/lib/libandroid_runtime.so
(_ZN7android6Parcel17FlattenableHelperINS_13GraphicBufferEE9unflattenEPKvjPKij+20)
01-01 00:00:52.713  1062  1062 F DEBUG   : #12 pc 00044c23
/system/lib/libbinder.so
(_ZNK7android6Parcel4readERNS0_26FlattenableHelperInterfaceE+338)
01-01 00:00:52.713  1062  1062 F DEBUG   : #13 pc 000460fd
/system/lib/libgui.so
(_ZN7android23BpGraphicBufferProducer13requestBufferEiPNS_2spINS_13GraphicBufferEEE+128)
01-01 00:00:52.713  1062  1062 F DEBUG   : #14 pc 00050fc7
/system/lib/libgui.so
(_ZN7android7Surface13dequeueBufferEPP19ANativeWindowBufferPi+322)
01-01 00:00:52.713  1062  1062 F DEBUG   : #15 pc c931
/system/lib/egl/libGLES_mesa.so (update_buffers+52)
01-01 00:00:52.713  1062  1062 F DEBUG   : #16 pc ca2d
/system/lib/egl/libGLES_mesa.so (droid_image_get_buffers+16)
01-01 00:00:52.713  1062  1062 F DEBUG   : #17 pc 00067523
/system/lib/dri/gallium_dri.so (dri2_allocate_textures+378)
01-01 00:00:52.713  1062  1062 F DEBUG   : #18 pc 000648b7
/system/lib/dri/gallium_dri.so (dri_st_framebuffer_validate+134)
01-01 00:00:52.714  1062  1062 F DEBUG   : #19 pc 00182237
/system/lib/dri/gallium_dri.so 

[Mesa-dev] [PATCH] etnaviv: fix refcnt initialization in etna_screen

[Aleksander Morgado]

Despite being a member of the etna_screen struct, 'refcnt' is used by
the winsys-specific logic to track the reference count of the object
managed in a hash table. When the count reaches zero, the pipe screen
is removed from the table and destroyed.

Fix the logic by initializing the refcnt to 1 when screen created.
This initialization is done in etna_screen_create(), to follow the
same logic as in freedreno and virgl.

Signed-off-by: Aleksander Morgado 
---
 src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index eefb51c5da..fa0cbd9076 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -772,6 +772,7 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu 
*gpu,
screen->dev = dev;
screen->gpu = gpu;
screen->ro = renderonly_dup(ro);
+   screen->refcnt = 1;
 
if (!screen->ro) {
   DBG("could not create renderonly object");
-- 
2.13.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev