kernel: media_request_get/put around inf->req (UAF safety)
Sonnet pre-deployment review flagged a SHIP-WITH-EYES-OPEN risk:
Phase 8.13's inf->req captured src_buf->vb2_buf.req_obj.req as a
raw pointer with no media_request_get(). On the normal decode
path that's fine because vb2-core holds its own reference until
v4l2_m2m_buf_done_and_job_finish releases it.
But on a concurrent cancel (MEDIA_IOC_REQUEST_REINIT or a process
kill triggering buf_request_complete from the cancel path before
RESP_FRAME comes back), vb2 could drop its reference first. Our
inf->req would then dangle through v4l2_ctrl_request_complete +
buf_done_and_job_finish — UAF.
Fix matches the cedrus / rkvdec pattern: take our own reference
when we capture the pointer, release it after we're done with it
(after buf_done_and_job_finish to keep the ordering crystal-clear).
/* in daedalus_device_run, after inf->req = src_buf->...->req */
if (inf->req)
media_request_get(inf->req);
/* in daedalus_complete_resp_frame, after buf_done_and_job_finish */
if (inf->req)
media_request_put(inf->req);
Verified on hertz:
- libva path (request-bound, inf->req != NULL): byte-exact NV12,
same FNV-1a as standalone.
- test_m2m_stream (direct QBUF, inf->req == NULL): 30/30 frames
decoded, conditional skip works.
- No kernel oops / WARN, no leak in dmesg.
Add #include <media/media-request.h> for the helpers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -44,6 +44,7 @@
|
|||||||
#include <media/v4l2-event.h>
|
#include <media/v4l2-event.h>
|
||||||
#include <media/media-device.h>
|
#include <media/media-device.h>
|
||||||
#include <media/media-entity.h>
|
#include <media/media-entity.h>
|
||||||
|
#include <media/media-request.h>
|
||||||
#include <media/videobuf2-v4l2.h>
|
#include <media/videobuf2-v4l2.h>
|
||||||
#include <media/videobuf2-dma-contig.h>
|
#include <media/videobuf2-dma-contig.h>
|
||||||
|
|
||||||
@@ -680,8 +681,18 @@ static void daedalus_device_run(void *priv)
|
|||||||
* trigger MEDIA_REQUEST_STATE_COMPLETE. vb2-core's normal
|
* trigger MEDIA_REQUEST_STATE_COMPLETE. vb2-core's normal
|
||||||
* buf_done path unbinds the buffer's req_obj but leaves the
|
* buf_done path unbinds the buffer's req_obj but leaves the
|
||||||
* control object bound — the driver has to complete it.
|
* control object bound — the driver has to complete it.
|
||||||
|
*
|
||||||
|
* Take our own reference via media_request_get so the
|
||||||
|
* pointer stays valid even if vb2 releases its reference
|
||||||
|
* concurrently (e.g. via MEDIA_IOC_REQUEST_REINIT or a
|
||||||
|
* process kill triggering buf_request_complete from the
|
||||||
|
* cancel path). Released by media_request_put in
|
||||||
|
* daedalus_complete_resp_frame. Matches the cedrus /
|
||||||
|
* rkvdec refcount pattern.
|
||||||
*/
|
*/
|
||||||
inf->req = src_buf->vb2_buf.req_obj.req;
|
inf->req = src_buf->vb2_buf.req_obj.req;
|
||||||
|
if (inf->req)
|
||||||
|
media_request_get(inf->req);
|
||||||
|
|
||||||
mutex_lock(&dev->inflight_lock);
|
mutex_lock(&dev->inflight_lock);
|
||||||
list_add_tail(&inf->list, &dev->inflight);
|
list_add_tail(&inf->list, &dev->inflight);
|
||||||
@@ -833,6 +844,15 @@ void daedalus_complete_resp_frame(u32 cookie,
|
|||||||
v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev, inf->ctx->m2m_ctx,
|
v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev, inf->ctx->m2m_ctx,
|
||||||
state);
|
state);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Release our reference taken in device_run; safe to do
|
||||||
|
* AFTER buf_done_and_job_finish (which dropped the vb2
|
||||||
|
* reference) because we still hold this one. If the
|
||||||
|
* refcount hits zero here, media-core releases the request.
|
||||||
|
*/
|
||||||
|
if (inf->req)
|
||||||
|
media_request_put(inf->req);
|
||||||
|
|
||||||
kfree(inf);
|
kfree(inf);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user