linux-kernel - Re: [RFC 2/4] media: videobuf2: Replace bufs array by a list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 15 Mar 2023 09:57:51 -0400
From:   Nicolas Dufresne <nicolas.dufresne@...labora.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>
Cc:     tfiga@...omium.org, m.szyprowski@...sung.com, mchehab@...nel.org,
        ming.qian@....com, shijie.qin@....com, eagle.zhou@....com,
        bin.liu@...iatek.com, matthias.bgg@...il.com,
        angelogioacchino.delregno@...labora.com, tiffany.lin@...iatek.com,
        andrew-ct.chen@...iatek.com, yunfei.dong@...iatek.com,
        stanimir.k.varbanov@...il.com, quic_vgarodia@...cinc.com,
        agross@...nel.org, andersson@...nel.org, konrad.dybcio@...aro.org,
        ezequiel@...guardiasur.com.ar, p.zabel@...gutronix.de,
        daniel.almeida@...labora.com, hverkuil-cisco@...all.nl,
        jerbel@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
        linux-rockchip@...ts.infradead.org, kernel@...labora.com
Subject: Re: [RFC 2/4] media: videobuf2: Replace bufs array by a list

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.

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 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.