[Mesa-dev] [PATCH 1/3] cso: don't release sampler states that are bound

[Marek Olšák]

On Mon, Dec 5, 2016 at 10:05 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 03/12/16 05:38 AM, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> This fixes random radeonsi GPU hangs in Batman Arkham: Origins (Wine) and
>> probably many other games too.
>>
>> cso_cache deletes sampler states when the cache size is too big and doesn't
>> check which sampler states are bound, causing use-after-free in drivers.
>> Because of that, radeonsi uploaded garbage sampler states and the hardware
>> went bananas. Other drivers may have experienced similar issues.
>>
>> Cc: 13.0 12.0 <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/gallium/auxiliary/cso_cache/cso_cache.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/cso_cache/cso_cache.c b/src/gallium/auxiliary/cso_cache/cso_cache.c
>> index b240c93..1f3be4b 100644
>> --- a/src/gallium/auxiliary/cso_cache/cso_cache.c
>> +++ b/src/gallium/auxiliary/cso_cache/cso_cache.c
>> @@ -181,21 +181,23 @@ static inline void sanitize_cb(struct cso_hash *hash, enum cso_cache_type type,
>>        --to_remove;
>>     }
>>  }
>>
>>  struct cso_hash_iter
>>  cso_insert_state(struct cso_cache *sc,
>>                   unsigned hash_key, enum cso_cache_type type,
>>                   void *state)
>>  {
>>     struct cso_hash *hash = _cso_hash_for_type(sc, type);
>> -   sanitize_hash(sc, hash, type, sc->max_size);
>> +
>> +   if (type != CSO_SAMPLER)
>> +      sanitize_hash(sc, hash, type, sc->max_size);
>>
>>     return cso_hash_insert(hash, hash_key, state);
>>  }
>>
>>  struct cso_hash_iter
>>  cso_find_state(struct cso_cache *sc,
>>                 unsigned hash_key, enum cso_cache_type type)
>>  {
>>     struct cso_hash *hash = _cso_hash_for_type(sc, type);
>>
>>
>
> If I understand correctly, this means that the maximum cache size
> effectively isn't enforced for sampler states? Wouldn't it be better
> instead to add code which prevents currently bound states from being
> deleted from here?

Not in this patch. Maybe later and if CPU profiling results show that
it matters.

Currently, if the cache size is exceeded, the GPU usually hangs. If
pruning the cache was important, most apps would hang.

Thus, I'd like to push this as-is.

Marek