2015-02-25 07:29:36

by Sudip JAIN

[permalink] [raw]
Subject: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch

Dear Maintainer,

PFA attached patch that prevents user from being returned garbage bytesused value from vb2 framework.

Regards,
Sudip Jain


Attachments:
0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch (1.60 kB)
0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch

2015-02-25 18:23:14

by Jeremiah Mahler

[permalink] [raw]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch

Sudip,

On Wed, Feb 25, 2015 at 03:29:22PM +0800, Sudip JAIN wrote:
> Dear Maintainer,
>
> PFA attached patch that prevents user from being returned garbage bytesused value from vb2 framework.
>
> Regards,
> Sudip Jain
>

Patches should never be submitted as attachments, they should be inline.

See Documentation/SubmittingPatches for more info.

[...]

--
- Jeremiah Mahler

2015-02-25 19:45:11

by Jean-Michel Hautbois

[permalink] [raw]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch

Hi Sudip,

2015-02-25 19:23 GMT+01:00 Jeremiah Mahler <[email protected]>:
> Sudip,
>
> On Wed, Feb 25, 2015 at 03:29:22PM +0800, Sudip JAIN wrote:
>> Dear Maintainer,
>>
>> PFA attached patch that prevents user from being returned garbage bytesused value from vb2 framework.
>>
>> Regards,
>> Sudip Jain
>>
>
> Patches should never be submitted as attachments, they should be inline.
>
> See Documentation/SubmittingPatches for more info.

You also can have a look here :
http://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches

In fact, is the patch on top of master tree ? According to line
numbers, I would say no.

Thx,
JM

2015-02-26 05:18:45

by Sudip JAIN

[permalink] [raw]
Subject: RE: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch; kernel version 3.10.69

Hello Jeremiah,

Please find the patch "inline"

commit 3390900680e5182998916c8fa231bc79cd84046b
Author: Sudip Jain <[email protected]>
Date: Thu Feb 26 10:40:34 2015 +0530

media: vb2: Fill vb2_buffer with bytesused from user

In vb2_qbuf for dmabuf memory type, userside bytesused is not read to
vb2 buffer. This leads garbage value being copied from __qbuf_dmabuf()
back to user in __fill_v4l2_buffer().

As a default case, the vb2 framework must trust the userside value,
and also allow driver's buffer prepare function prefer modify/update
or not to.

Applied on kernel version 3.10.69

Change-Id: Ieda389403898935f59c2e2994106f3e5238cfefd
Signed-off-by: Sudip Jain <[email protected]>

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 5e47ba4..54fe9c9 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -919,6 +919,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
b->m.planes[plane].m.fd;
v4l2_planes[plane].length =
b->m.planes[plane].length;
+ v4l2_planes[plane].bytesused =
+ b->m.planes[plane].bytesused;
v4l2_planes[plane].data_offset =
b->m.planes[plane].data_offset;
}
@@ -943,6 +945,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
if (b->memory == V4L2_MEMORY_DMABUF) {
v4l2_planes[0].m.fd = b->m.fd;
v4l2_planes[0].length = b->length;
+ v4l2_planes[0].bytesused = b->bytesused;
v4l2_planes[0].data_offset = 0;
}

Thanks,
Sudip
________________________________________
From: Jeremiah Mahler [[email protected]]
Sent: Wednesday, February 25, 2015 11:53 PM
To: Sudip JAIN
Cc: [email protected]; [email protected]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch

Sudip,

On Wed, Feb 25, 2015 at 03:29:22PM +0800, Sudip JAIN wrote:
> Dear Maintainer,
>
> PFA attached patch that prevents user from being returned garbage bytesused value from vb2 framework.
>
> Regards,
> Sudip Jain
>

Patches should never be submitted as attachments, they should be inline.

See Documentation/SubmittingPatches for more info.

[...]

--
- Jeremiah Mahler

2015-02-26 08:58:22

by Hans Verkuil

[permalink] [raw]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch; kernel version 3.10.69

Hi Jeremiah,

On 02/26/15 06:18, Sudip JAIN wrote:
> Hello Jeremiah,
>
> Please find the patch "inline"
>
> commit 3390900680e5182998916c8fa231bc79cd84046b
> Author: Sudip Jain <[email protected]>
> Date: Thu Feb 26 10:40:34 2015 +0530
>
> media: vb2: Fill vb2_buffer with bytesused from user
>
> In vb2_qbuf for dmabuf memory type, userside bytesused is not read to
> vb2 buffer. This leads garbage value being copied from __qbuf_dmabuf()
> back to user in __fill_v4l2_buffer().
>
> As a default case, the vb2 framework must trust the userside value,
> and also allow driver's buffer prepare function prefer modify/update
> or not to.
>
> Applied on kernel version 3.10.69

This kernel is way, way too old. If you provide a patch then you should
use the latest released kernel, or, even better, the master branch of the
media development git repository: http://git.linuxtv.org/cgit.cgi/media_tree.git/

Also, bytesused only needs to be set for output buffers, never for capture
buffers: the driver will set bytesused when a frame is captured. So this
patch makes no sense.

Regards,

Hans

>
> Change-Id: Ieda389403898935f59c2e2994106f3e5238cfefd
> Signed-off-by: Sudip Jain <[email protected]>
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 5e47ba4..54fe9c9 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -919,6 +919,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> b->m.planes[plane].m.fd;
> v4l2_planes[plane].length =
> b->m.planes[plane].length;
> + v4l2_planes[plane].bytesused =
> + b->m.planes[plane].bytesused;
> v4l2_planes[plane].data_offset =
> b->m.planes[plane].data_offset;
> }
> @@ -943,6 +945,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> if (b->memory == V4L2_MEMORY_DMABUF) {
> v4l2_planes[0].m.fd = b->m.fd;
> v4l2_planes[0].length = b->length;
> + v4l2_planes[0].bytesused = b->bytesused;
> v4l2_planes[0].data_offset = 0;
> }
>
> Thanks,
> Sudip

2015-02-26 17:09:16

by Jeremiah Mahler

[permalink] [raw]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch; kernel version 3.10.69

Sudip,

On Thu, Feb 26, 2015 at 01:18:31PM +0800, Sudip JAIN wrote:
> Hello Jeremiah,
>
> Please find the patch "inline"
>
There are more problems than just not being "inline".

- The subject line wrong.
- The log message is formatted wrong.
- This patch is not in a form that can be applied.
- And there are probably more problems as well...

I would review how to submit patches and then try again.
I recommend watching the video by Greg Kroah-Hartman on how
to submit your first kernel patch [1].

[1]: https://www.youtube.com/watch?v=LLBrBBImJt4

[...]

--
- Jeremiah Mahler

2015-02-27 06:26:55

by Sudip JAIN

[permalink] [raw]
Subject: RE: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch; kernel version 3.10.69

Thanks for your feedback. I will improve my patch submission.

BR,
Sudip
________________________________________
From: Jeremiah Mahler [[email protected]]
Sent: Thursday, February 26, 2015 10:39 PM
To: Sudip JAIN
Cc: [email protected]; [email protected]
Subject: Re: 0001-media-vb2-Fill-vb2_buffer-with-bytesused-from-user.patch; kernel version 3.10.69

Sudip,

On Thu, Feb 26, 2015 at 01:18:31PM +0800, Sudip JAIN wrote:
> Hello Jeremiah,
>
> Please find the patch "inline"
>
There are more problems than just not being "inline".

- The subject line wrong.
- The log message is formatted wrong.
- This patch is not in a form that can be applied.
- And there are probably more problems as well...

I would review how to submit patches and then try again.
I recommend watching the video by Greg Kroah-Hartman on how
to submit your first kernel patch [1].

[1]: https://www.youtube.com/watch?v=LLBrBBImJt4

[...]

--
- Jeremiah Mahler