On Fri, Jun 7, 2019 at 9:01 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > On 6/7/19 1:16 PM, Laurent Pinchart wrote: > > Hi Hans, > > > > Thank you for the patch. > > > > On Fri, Jun 07, 2019 at 10:45:31AM +0200, Hans Verkuil wrote: > >> The __prepare_userptr() function made the incorrect assumption that if the > >> same user pointer was used as the last one for which memory was acquired, then > >> there was no need to re-acquire the memory. This assumption was never properly > >> tested, and after doing that it became clear that this was in fact wrong. > > > > Could you explain in the commit message why the assumption is not > > correct ? > > You can free the memory, then allocate it again and you can get the same pointer, > even though it is not necessarily using the same physical pages for the memory > that the kernel is still using for it. > > Worse, you can free the memory, then allocate only half the memory you need and > get back the same pointer. vb2 wouldn't notice this. And it seems to work (since > the original mapping still remains), but this can corrupt userspace memory > causing the application to crash. It's not quite clear to me how the memory can > get corrupted. I don't know enough of those low-level mm internals to understand > the sequence of events. Chrome specifically didn't keep the mapping between user pointers and indexes, so it the cache just missed every time. What we noticed was the put_userptr on the previous userptr at the index being unmapped apparently caused that memory (often already returned back to the application) to be corrupted... But we didn't get to the bottom of it either, as we didn't have any MM expert look at the issue. The free and realloc scenario just came to my mind when trying to recall our original problem earlier today. Best regards, Tomasz
