From f55b2cd002afdfd08f3c093627317f92e4929074 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Mon, 18 May 2026 18:39:10 +0000 Subject: [PATCH] kernel: media_request_get/put around inf->req (UAF safety) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 for the helpers. Co-Authored-By: Claude Opus 4.7 (1M context) --- kernel/daedalus_v4l2_main.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/kernel/daedalus_v4l2_main.c b/kernel/daedalus_v4l2_main.c index 6dfd6cf..f3830a5 100644 --- a/kernel/daedalus_v4l2_main.c +++ b/kernel/daedalus_v4l2_main.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include @@ -680,8 +681,18 @@ static void daedalus_device_run(void *priv) * trigger MEDIA_REQUEST_STATE_COMPLETE. vb2-core's normal * buf_done path unbinds the buffer's req_obj but leaves the * 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; + if (inf->req) + media_request_get(inf->req); mutex_lock(&dev->inflight_lock); 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, 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); }