On Wed, Mar 15, 2023 at 09:57:51AM -0400, Nicolas Dufresne wrote: > Le lundi 13 mars 2023 à 20:11 +0200, Laurent Pinchart a écrit : > > > - /* Ensure that q->num_buffers+num_buffers is below VB2_MAX_FRAME */ > > > - num_buffers = min_t(unsigned int, num_buffers, > > > - VB2_MAX_FRAME - q->num_buffers); > > > - > > > > We can indeed drop this check now, but shouldn't we introduce some kind > > of resource accounting and limitation ? Otherwise any unpriviledged > > userspace will be able to starve system memory. This could be > > implemented on top, as the problem largely exists today already, but I'd > > like to at least record this in a TODO comment. > > The current limit already isn't work for resource accounting and limitation for > m2m drivers. You can open a device, allocate 32 buffers, and close that device > keeping the memory around. And redo this process as many times as you want. I know, that's why I mentioned that the problem largely exists today already. > A TODO is most appropriate, but I would prefer to see this done at a memory > layer level (rather then v4l2 specific), so that limits and accounting works > with containers and other sandboxes. I haven't thought about how this could be implemented, all I know is that it's about time to tackle this issue, so I would like to at least record it. > > I also wonder if we should still limit the number of allocated buffers. > > The limit could be large, for instance 1024 buffers, and it would be an > > in-kernel limit that could be increased later if needed. I'm concerned > > that dropping the limit completely will allow userspace to request > > UINT_MAX buffers, which may cause integer overflows somewhere. Limiting > > the number of buffers would avoid extensive review of all the code that > > deals with counting buffers. > -- Regards, Laurent Pinchart
