[vlc-devel] [PATCH v3 04/10] core: use refcounter helper for pictures

[Romain Vimont]

On Fri, Jul 06, 2018 at 07:40:51PM +0300, Rémi Denis-Courmont wrote:
> Le keskiviikkona 4. heinäkuuta 2018, 12.17.58 EEST Romain Vimont a écrit :
> > On Wed, Jul 04, 2018 at 12:02:45PM +0300, Rémi Denis-Courmont wrote:
> > > I never said that uint was enough for anything
> > 
> > Is uint not enough for normal usage (some objects are intended to be
> > refcounted many many times)
> 
> By that argument, you could use unsigned int instead of size_t and yet you 
> don't. I don't see how you can ensure less than 1<<32 references on 64-bits 
> system in the general case.
>
> > or not enough as a security reason against malicious usage?
> 
> Again, the whole point of Elena's refcount_t in kernel is that 32-bits 
> overflow causes security issues, notably use after free.
> 
> I don't know why the kernel decided to use more complex code to prevent 
> overflow, rather than simply switch to ulong. Probably because of 32-bits 
> platforms and/or to save comparatively expensive kernel memory.

The hardening against refcount overflow is intented to protect against
exploitation (by use-after-free) of bugs, like a missing refcount
decrement, so a malicious program could create many instances to make it
overflow and exploit the bug.

<https://lwn.net/Articles/693038/>

However, IMO, willingly keeping 1<<32 refs to an object is never
expected (it would be a design bug).

We could decide to harden refcounts (this is not the purpose of this
patchset). In that case, it should also apply to picture pool: the
number of refs is expected to be bounded, but refcount overflow protects
against exploitation of bugs.