From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=209.85.167.53 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793703372; b=KB4xfQZVY7FRpIdC5RGXWti+uD7PnIM19gjuyO/G4edia02WdOFPrbes5tc023afIERQ yFN84p2ihsinH/ou/MUi7I/hewGjtYVZrFPnIwLlJNC829l77C+clX2rlFnzajLkHgj+D /ZdNBpMhq9RdHy3Gt3t8YDh8ZOrAJlPwogioJTOTM7dg0z8QkpOszgNvr6MyKdxQ6h7D8 2lM8R7xZSOjP2FADsSD5ruPDRDAdmXRNsrFOKzGzlguGL2F2o8uHcJr4EjauQE9gEV06K RqUz/YBIR03N33ysqbu6CnQ+7OkoZIdNl7SkRsJBtxxP7KD4Bn7jb4KH7vQJPuPdwSg== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793703372; h=DMARC-Filter:Received:DKIM-Signature:X-Google-DKIM-Signature: X-Forwarded-Encrypted:X-Gm-Message-State:X-Gm-Gg:X-Received:Received: From:Date:Subject:MIME-Version:Content-Type:Content-Transfer-Encoding: Message-Id:References:In-Reply-To:To:Cc:X-Mailer; bh=xSD7cxthB9W0KI/LsHWzLh21tuW/dkbAwJgVNf3RpSU=; b=LrwpmF0eljxH11+DbxlEBbeE/QtYwVtMiGSEM62jIz39v1eRD27zU5hgTfyDbHde5m1P T2qzjMQeOmtjpSIZKHScWmuRDw16dtGnfEDfXmR6KRJ9MYIoe0CmOdYsh1jMsY6425QS5 sfWIpQ2qyRU0qIXhT9jY9c2OCfH6Hbq7112vK1xFXwMZyJqlWJj+yYlkAxy0czTpPL964 zJiwqctQZgFlqPzXeqaUSHU073D1uw/IYZgKBjroGa8mAuQVYKkvDduXUQQf6YtlxX36b RsmBoaHGw7jaqo3DQBftAo6NYmX9cz8ydUcTL7bIFH98a8iOCQNUIIWmndhVyMlZmIg== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=CMshHRoQ; arc=none smtp.remote-ip=209.85.167.53 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 71A7D606EEAB for ; Tue, 3 Nov 2026 10:56:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org 71A7D606EEAB Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key, unprotected) header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=CMshHRoQ Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-5bfa65c8617so2405577e87.0 for ; Tue, 03 Nov 2026 02:56:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1793703371; x=1794308171; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :content-type:mime-version:subject:date:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=xSD7cxthB9W0KI/LsHWzLh21tuW/dkbAwJgVNf3RpSU=; b=CMshHRoQ9LA4EEdoYu9Y+zoBFGOFRgQcTHYuyrnsAZKZYPEWCpOESF/AL1aDG/CfTs vkSiQc360/qvZc6nKAo7LoaKhZmtPp9S1JBC+FslRc9n6m5ZIjNR9ihE3gwrQ4x3wMjm 1V19ifHiEk4sWrAPU6zhVEZ//ylGrLSgAUvjQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1793703371; x=1794308171; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :content-type:mime-version:subject:date:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=xSD7cxthB9W0KI/LsHWzLh21tuW/dkbAwJgVNf3RpSU=; b=kyGeKyg375SmP1FKhqtH8T7RYHyRzAjlpY4jQ90uQpR0gXs5pQXzWdl53sZuH2uRcL pkfItL4uu+WQSBcvRwReTM8PKpPX00iJ9wJoaTIGY0DoNtvd4Zt4DU46pS2a1MBddTEJ ll/ksZfjPoZEJnh+Z9bPiSCiQRwVKBB+bwTqBnR2VJAy8U9BXZ7y06NJ20P05Ngo2ypH WFc+Fo8jQVURQRRbTzazP++L4n8QuV0U3cqFBj5T7YxgI78MBYBhb8z5LLEHi1MSCy2L jVuHrgWhQzNr2fji6u6p4Hx/1NKYJXZVgycJIiyoZEGeQr2HKY1xAELaaMIiOho3Stp1 AyPQ== X-Forwarded-Encrypted: i=1; AKwUvByW6CbyAS4xYNgurbFZzw3T7VcJROWhY2nOMkBdyqIsSZ6DZdrVrdqOfixKsFzKRhuPta+1IRrTx2Xg9OY=@vger.kernel.org X-Gm-Message-State: APuU4Z+t8W1NEmXzPWXARhNKw8eCvAk9+rm/d4KBJI8etqoqx0ZBEuod SQz9xvCWZQZcPmd1G95ERxxgsTJ9D5e01QMM5asxcvB3cw5CsyS1W7MtbLmgHrl6vA== X-Gm-Gg: AYBFou1E2hF74DQkIPEGLpeW8yPpE7il6OhpfenvBDMxthjfWGAgPaPz3fmKHOFISOt A8GuNASoNsExN+FBYnRnTXFRrPqzVOhplJq+SKVlsCZO2yXPb3OcXEYMkwlrmw8/r7To9fxsbTN YsWZgCGYKITGdx4EXmRfxYQlwgE7+KOIbr4DsVJg+DeCRsbWqWeWinDyg5xve/gZ+Pu2+FfJOWt RxW8e9TWurlc6S8gpShOic2hfNQ3aOhUKXbV570lXK4Hv4nqRSbgcMp7QMuQmBgSh9qIjhpMLl5 izMfYERVJZoekGKRLjBX8MivxgnjKfmqw5BAy3Qou+P9PRslWO1PnbWuSWWqr/MXacc4Ig+Uhgk qbcKKCHhdlkaChBtHI9Z7cWZOQcRLVxw4snndbKUOo0doD+iCE7s3zrJkCpHII6xXBmsryDUDvL dQDvABTl/UtJkUHG82Jzci2Bk+8NWVa2NOV+X3PmGGBUUYRS7N6NHT9zg+qky9jBRBjNOO4wREV D90MYyebuEqzZtKLmuRHK2fZJby+xSPNhIOQJMU/E4bNvseotHc0tGt1XED3p1zuC/kPB+yugNO tXRpF3junrZgH7M5+rZqx2osCsG3W3+G1wEqgZWl4eEvBzVTjDIVH7gVYsFAUXkKydyoEWd8c3o sSI0k46OxHzc= X-Received: by 2002:a05:6512:398c:b0:5ba:3300:d521 with SMTP id 2adb3069b0e04-5bfd957f357mr3782618e87.41.1793703370702; Tue, 03 Nov 2026 02:56:10 -0800 (PST) Received: from ribalda.c.googlers.com (214.118.88.34.bc.googleusercontent.com. [34.88.118.214]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5bfe0754779sm2103257e87.82.2026.11.03.02.56.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Nov 2026 02:56:10 -0800 (PST) From: Ricardo Ribalda Date: Tue, 03 Nov 2026 10:56:08 +0000 Subject: [PATCH v2 1/2] media: videobuf2: publish buffers in bufs_bitmap under mmap_lock X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20261103-vb2-mmap_lock-v2-1-054e11dcd013@chromium.org> References: <20261103-vb2-mmap_lock-v2-0-054e11dcd013@chromium.org> In-Reply-To: <20261103-vb2-mmap_lock-v2-0-054e11dcd013@chromium.org> To: Tomasz Figa , Marek Szyprowski , Mauro Carvalho Chehab , Hans Verkuil , Benjamin Gaignard Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Ricardo Ribalda X-Mailer: b4 0.14.3 vb2_mmap() only takes q->mmap_lock, not the main queue lock (q->lock). To keep vb2_mmap() safe, newly allocated buffers must not become visible in vb2_get_buffer() until they are fully initialized and q->mmap_lock is held. Commit a286b0837e13 ("media: core: Add bitmap manage bufs array entries") started setting q->bufs_bitmap inside __vb2_queue_alloc() before buffer memory is allocated and without holding q->mmap_lock. As a result: - A concurrent mmap() can find a half-initialized buffer, and if buffer allocation or .buf_init() fails, __vb2_queue_alloc() frees that buffer without holding q->mmap_lock, causing a use-after-free in vb2_mmap(). - If fewer buffers are allocated than requested, they are already counted as active in q->bufs_bitmap before .queue_setup() checks whether the smaller count is acceptable. Fix this by keeping new buffers only in q->bufs[] during allocation and setting their bits in q->bufs_bitmap under q->mmap_lock once allocation and .queue_setup() succeed, and let __vb2_queue_free() read q->bufs[] directly when cleaning up unexposed buffers. Fixes: a286b0837e13 ("media: core: Add bitmap manage bufs array entries") Cc: stable@vger.kernel.org Signed-off-by: Ricardo Ribalda --- drivers/media/common/videobuf2/videobuf2-core.c | 66 +++++++++++++++++++++---- include/media/videobuf2-core.h | 8 +-- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index b0a6084f1757..70ef38c3d16a 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -431,6 +431,9 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb) * @q: pointer to &struct vb2_queue with videobuf2 queue. * @vb: pointer to &struct vb2_buffer to be added to the queue. * @index: index where add vb2_buffer in the queue + * + * The buffer is stored in @q->bufs[] but remains invisible to vb2_get_buffer() + * until published with vb2_queue_publish_buffers(). */ static void vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, unsigned int index) { @@ -439,7 +442,31 @@ static void vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, uns q->bufs[index] = vb; vb->index = index; vb->vb2_queue = q; - set_bit(index, q->bufs_bitmap); +} + +/** + * vb2_queue_publish_buffers() - publish buffers staged by __vb2_queue_alloc() + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * @start: index of the first buffer to publish. + * @count: number of buffers to publish. + * + * Set the @q->bufs_bitmap bits of the @count fully initialized buffers that + * __vb2_queue_alloc() stored in @q->bufs[] starting at @start, which makes + * them visible to vb2_get_buffer(). + */ +static void vb2_queue_publish_buffers(struct vb2_queue *q, unsigned int start, + unsigned int count) +{ + unsigned int i; + + lockdep_assert_held(&q->mmap_lock); + + for (i = start; i < start + count; i++) { + if (WARN_ON(i >= q->max_num_buffers || !q->bufs[i] || + test_bit(i, q->bufs_bitmap))) + continue; + set_bit(i, q->bufs_bitmap); + } } /** @@ -460,6 +487,11 @@ static void vb2_queue_remove_buffer(struct vb2_buffer *vb) * @first_index: index of the first created buffer, all newly allocated buffers * have indices in the range [first_index..first_index+count-1] * + * The allocated buffers are stored in q->bufs[] but not published in + * q->bufs_bitmap: the caller must either publish them with + * vb2_queue_publish_buffers() or free them with __vb2_queue_free(), both under + * q->mmap_lock. + * * Returns the number of buffers successfully allocated. */ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, @@ -562,7 +594,8 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int start, unsigned int struct vb2_buffer *vb; for (i = start; i < start + count; i++) { - vb = vb2_get_buffer(q, i); + /* See __vb2_queue_free() for why vb2_get_buffer() isn't used */ + vb = q->bufs[i]; if (!vb) continue; @@ -580,6 +613,11 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int start, unsigned int * __vb2_queue_free() - free @count buffers from @start index of the queue - video memory and * related information, if no buffers are left return the queue to an * uninitialized state. Might be called even if the queue has already been freed. + * + * Buffers are looked up directly in q->bufs[] instead of through + * vb2_get_buffer() so that buffers staged by __vb2_queue_alloc() that were + * never published in q->bufs_bitmap (because a later .queue_setup() call + * rejected the allocated number of buffers) are freed as well. */ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned int count) { @@ -587,9 +625,13 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned i lockdep_assert_held(&q->mmap_lock); + /* q->bufs is NULL until vb2_core_allocated_buffers_storage() runs */ + if (!q->bufs) + return; + /* Call driver-provided cleanup function for each buffer, if provided */ for (i = start; i < start + count; i++) { - struct vb2_buffer *vb = vb2_get_buffer(q, i); + struct vb2_buffer *vb = q->bufs[i]; if (vb && vb->planes[0].mem_priv) call_void_vb_qop(vb, buf_cleanup, vb); @@ -624,7 +666,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned i q->cnt_unprepare_streaming = 0; } for (i = start; i < start + count; i++) { - struct vb2_buffer *vb = vb2_get_buffer(q, i); + struct vb2_buffer *vb = q->bufs[i]; bool unbalanced; if (!vb) @@ -676,7 +718,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned i /* Free vb2 buffers */ for (i = start; i < start + count; i++) { - struct vb2_buffer *vb = vb2_get_buffer(q, i); + struct vb2_buffer *vb = q->bufs[i]; if (!vb) continue; @@ -1000,14 +1042,16 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, if (ret < 0) { /* - * Note: __vb2_queue_free() will subtract 'allocated_buffers' - * from already queued buffers and it will reset q->memory to + * Note: the buffers allocated by __vb2_queue_alloc() were never + * published in q->bufs_bitmap, __vb2_queue_free() frees them + * straight from q->bufs[] and resets q->memory to * VB2_MEMORY_UNKNOWN. */ __vb2_queue_free(q, first_index, allocated_buffers); mutex_unlock(&q->mmap_lock); return ret; } + vb2_queue_publish_buffers(q, first_index, allocated_buffers); mutex_unlock(&q->mmap_lock); /* @@ -1125,14 +1169,16 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, if (ret < 0) { /* - * Note: __vb2_queue_free() will subtract 'allocated_buffers' - * from already queued buffers and it will reset q->memory to - * VB2_MEMORY_UNKNOWN. + * Note: the buffers allocated by __vb2_queue_alloc() were never + * published in q->bufs_bitmap, __vb2_queue_free() frees them + * straight from q->bufs[] and resets q->memory to + * VB2_MEMORY_UNKNOWN if no buffers are left. */ __vb2_queue_free(q, *first_index, allocated_buffers); mutex_unlock(&q->mmap_lock); return -ENOMEM; } + vb2_queue_publish_buffers(q, *first_index, allocated_buffers); mutex_unlock(&q->mmap_lock); /* diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 4b4f4c15c53a..2e93e6985d5f 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -564,9 +564,11 @@ struct vb2_buf_ops { * @mmap_lock: private mutex used when buffers are allocated/freed/mmapped * @memory: current memory type used * @dma_dir: DMA mapping direction. - * @bufs: videobuf2 buffer structures. If it is non-NULL then - * bufs_bitmap is also non-NULL. - * @bufs_bitmap: bitmap tracking whether each bufs[] entry is used + * @bufs: videobuf2 buffer structures. Must be accessed with + * vb2_get_buffer(), as an allocated buffer cannot be used + * until it is published. + * @bufs_bitmap: bitmap tracking published buffers. Bits are set and cleared + * under @mmap_lock. * @max_num_buffers: upper limit of number of allocated/used buffers. * If set to 0 v4l2 core will change it VB2_MAX_FRAME * for backward compatibility. -- 2.56.0.385.gac671c819a-goog