media: dvb_vb2: fix possible out of bound access
Commit Message
vb2_core_qbuf and vb2_core_querybuf don't check the range of b->index
controlled by the user.
Fix this by adding range checking code before using them.
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
---
drivers/media/dvb-core/dvb_vb2.c | 11 +++++++++++
1 file changed, 11 insertions(+)
Comments
On (22/03/24 16:01), Hangyu Hua wrote:
> vb2_core_qbuf and vb2_core_querybuf don't check the range of b->index
> controlled by the user.
>
> Fix this by adding range checking code before using them.
>
> Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
On 2022/3/24 18:23, Sergey Senozhatsky wrote:
> On (22/03/24 16:01), Hangyu Hua wrote:
>> vb2_core_qbuf and vb2_core_querybuf don't check the range of b->index
>> controlled by the user.
>>
>> Fix this by adding range checking code before using them.
>>
>> Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
>
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Hi guys,
Two other patches for this module that I submitted at the same time as
this one have been accepted. But this bug looks much worse than the
other two. Is this patch forgotten to merge?
Thanks.
Hi Hangyu,
It appears this patch fell through the cracks. It's certainly useful to
have.
On 3/24/22 09:01, Hangyu Hua wrote:
> vb2_core_qbuf and vb2_core_querybuf don't check the range of b->index
> controlled by the user.
>
> Fix this by adding range checking code before using them.
>
> Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
> ---
> drivers/media/dvb-core/dvb_vb2.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
> index a1bd6d9c9223..f410800b92e7 100644
> --- a/drivers/media/dvb-core/dvb_vb2.c
> +++ b/drivers/media/dvb-core/dvb_vb2.c
> @@ -354,6 +354,12 @@ int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
>
> int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> {
> + struct vb2_queue *q = &ctx->vb_q;
> +
> + if (b->index >= q->num_buffers) {
> + dprintk(q, 1, "buffer index out of range\n");
However, this patch doesn't compile: dprintk in this source doesn't
have a 'q' argument!
> + return -EINVAL;
> + }
> vb2_core_querybuf(&ctx->vb_q, b->index, b);
> dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
Also, to help debugging it prefixes the warnings with the ctx-name.
Can you post a v2 of this patch?
Regards,
Hans
> return 0;
> @@ -378,8 +384,13 @@ int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
>
> int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> {
> + struct vb2_queue *q = &ctx->vb_q;
> int ret;
>
> + if (b->index >= q->num_buffers) {
> + dprintk(q, 1, "buffer index out of range\n");
> + return -EINVAL;
> + }
> ret = vb2_core_qbuf(&ctx->vb_q, b->index, b, NULL);
> if (ret) {
> dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
On 2022/5/18 18:44, Hans Verkuil wrote:
> Hi Hangyu,
>
> It appears this patch fell through the cracks. It's certainly useful to
> have.
>
> On 3/24/22 09:01, Hangyu Hua wrote:
>> vb2_core_qbuf and vb2_core_querybuf don't check the range of b->index
>> controlled by the user.
>>
>> Fix this by adding range checking code before using them.
>>
>> Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
>> ---
>> drivers/media/dvb-core/dvb_vb2.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
>> index a1bd6d9c9223..f410800b92e7 100644
>> --- a/drivers/media/dvb-core/dvb_vb2.c
>> +++ b/drivers/media/dvb-core/dvb_vb2.c
>> @@ -354,6 +354,12 @@ int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
>>
>> int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
>> {
>> + struct vb2_queue *q = &ctx->vb_q;
>> +
>> + if (b->index >= q->num_buffers) {
>> + dprintk(q, 1, "buffer index out of range\n");
>
> However, this patch doesn't compile: dprintk in this source doesn't
> have a 'q' argument!
Oops, i will fix this out.
>
>> + return -EINVAL;
>> + }
>> vb2_core_querybuf(&ctx->vb_q, b->index, b);
>> dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
>
> Also, to help debugging it prefixes the warnings with the ctx-name.
>
> Can you post a v2 of this patch?
>
> Regards,
>
> Hans
I will send a v2 later.
Thanks,
Hangyu
>
>> return 0;
>> @@ -378,8 +384,13 @@ int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
>>
>> int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
>> {
>> + struct vb2_queue *q = &ctx->vb_q;
>> int ret;
>>
>> + if (b->index >= q->num_buffers) {
>> + dprintk(q, 1, "buffer index out of range\n");
>> + return -EINVAL;
>> + }
>> ret = vb2_core_qbuf(&ctx->vb_q, b->index, b, NULL);
>> if (ret) {
>> dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
@@ -354,6 +354,12 @@ int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
{
+ struct vb2_queue *q = &ctx->vb_q;
+
+ if (b->index >= q->num_buffers) {
+ dprintk(q, 1, "buffer index out of range\n");
+ return -EINVAL;
+ }
vb2_core_querybuf(&ctx->vb_q, b->index, b);
dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
return 0;
@@ -378,8 +384,13 @@ int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
{
+ struct vb2_queue *q = &ctx->vb_q;
int ret;
+ if (b->index >= q->num_buffers) {
+ dprintk(q, 1, "buffer index out of range\n");
+ return -EINVAL;
+ }
ret = vb2_core_qbuf(&ctx->vb_q, b->index, b, NULL);
if (ret) {
dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,