* [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 [not found] <CGME20200417025205epcas2p46d33e64f2de49041d2ca68ecc98fc83e@epcas2p4.samsung.com> @ 2020-04-17 2:45 ` sy0816.kang 2020-04-17 7:24 ` Mauro Carvalho Chehab 2020-04-17 8:35 ` Greg Kroah-Hartman 0 siblings, 2 replies; 10+ messages in thread From: sy0816.kang @ 2020-04-17 2:45 UTC (permalink / raw) To: mchehab Cc: sy0816.kang, Hans Verkuil, Arnd Bergmann, Greg Kroah-Hartman, Thomas Gleixner, linux-media, linux-kernel From: Sunyoung Kang <sy0816.kang@samsung.com> get_v4l2_buffer32() didn't copy reserved2 field from userspace to driver. So the reserved2 value is not received through compat-ioctl32 in driver. This patch copy reserved2 field of v4l2_buffer in get_v4l2_buffer32(). Signed-off-by: Sunyoung Kang <sy0816.kang@samsung.com> --- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index a99e82ec9ab6..e9b2b9c0ec9a 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -665,6 +665,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *p64, if (V4L2_TYPE_IS_OUTPUT(type)) if (assign_in_user(&p64->bytesused, &p32->bytesused) || assign_in_user(&p64->field, &p32->field) || + assign_in_user(&p64->reserved2, &p32->reserved2) || assign_in_user(&p64->timestamp.tv_sec, &p32->timestamp.tv_sec) || assign_in_user(&p64->timestamp.tv_usec, -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-17 2:45 ` [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 sy0816.kang @ 2020-04-17 7:24 ` Mauro Carvalho Chehab 2020-04-18 3:13 ` Sunyoung Kang 2020-04-17 8:35 ` Greg Kroah-Hartman 1 sibling, 1 reply; 10+ messages in thread From: Mauro Carvalho Chehab @ 2020-04-17 7:24 UTC (permalink / raw) To: sy0816.kang Cc: Hans Verkuil, Arnd Bergmann, Greg Kroah-Hartman, Thomas Gleixner, linux-media, linux-kernel Em Fri, 17 Apr 2020 11:45:23 +0900 sy0816.kang@samsung.com escreveu: > From: Sunyoung Kang <sy0816.kang@samsung.com> > > get_v4l2_buffer32() didn't copy reserved2 field from userspace to driver. > So the reserved2 value is not received through compat-ioctl32 in driver. > This patch copy reserved2 field of v4l2_buffer in get_v4l2_buffer32(). Why should it copy reserved values? Those should not be used anywhere. > > Signed-off-by: Sunyoung Kang <sy0816.kang@samsung.com> > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index a99e82ec9ab6..e9b2b9c0ec9a 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -665,6 +665,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *p64, > if (V4L2_TYPE_IS_OUTPUT(type)) > if (assign_in_user(&p64->bytesused, &p32->bytesused) || > assign_in_user(&p64->field, &p32->field) || > + assign_in_user(&p64->reserved2, &p32->reserved2) || > assign_in_user(&p64->timestamp.tv_sec, > &p32->timestamp.tv_sec) || > assign_in_user(&p64->timestamp.tv_usec, Thanks, Mauro ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-17 7:24 ` Mauro Carvalho Chehab @ 2020-04-18 3:13 ` Sunyoung Kang 0 siblings, 0 replies; 10+ messages in thread From: Sunyoung Kang @ 2020-04-18 3:13 UTC (permalink / raw) To: 'Mauro Carvalho Chehab' Cc: 'Hans Verkuil', 'Arnd Bergmann', 'Greg Kroah-Hartman', 'Thomas Gleixner', linux-media, linux-kernel It uses the reserved 2 field to receive additional information about the buffer from the user. Additional information is for special functions. Copy the Reserved2 value to put_v4l2_buffer32(), but it is missing in get_v4l2_buffer32(). Can't I put it in get_v4l2_buffer32() also? Thanks, Sunyoung > -----Original Message----- > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Sent: Friday, April 17, 2020 4:24 PM > To: sy0816.kang@samsung.com > Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>; Arnd Bergmann <arnd@arndb.de>; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Thomas Gleixner > <tglx@linutronix.de>; linux-media@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in > get_v4l2_buffer32 > > Em Fri, 17 Apr 2020 11:45:23 +0900 > sy0816.kang@samsung.com escreveu: > > > From: Sunyoung Kang <sy0816.kang@samsung.com> > > > > get_v4l2_buffer32() didn't copy reserved2 field from userspace to driver. > > So the reserved2 value is not received through compat-ioctl32 in driver. > > This patch copy reserved2 field of v4l2_buffer in get_v4l2_buffer32(). > > Why should it copy reserved values? Those should not be used anywhere. > > > > > Signed-off-by: Sunyoung Kang <sy0816.kang@samsung.com> > > --- > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > index a99e82ec9ab6..e9b2b9c0ec9a 100644 > > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > > @@ -665,6 +665,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer > __user *p64, > > if (V4L2_TYPE_IS_OUTPUT(type)) > > if (assign_in_user(&p64->bytesused, &p32->bytesused) || > > assign_in_user(&p64->field, &p32->field) || > > + assign_in_user(&p64->reserved2, &p32->reserved2) || > > assign_in_user(&p64->timestamp.tv_sec, > > &p32->timestamp.tv_sec) || > > assign_in_user(&p64->timestamp.tv_usec, > > > > Thanks, > Mauro ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-17 2:45 ` [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 sy0816.kang 2020-04-17 7:24 ` Mauro Carvalho Chehab @ 2020-04-17 8:35 ` Greg Kroah-Hartman 2020-04-18 3:14 ` Sunyoung Kang 1 sibling, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2020-04-17 8:35 UTC (permalink / raw) To: sy0816.kang Cc: mchehab, Hans Verkuil, Arnd Bergmann, Thomas Gleixner, linux-media, linux-kernel On Fri, Apr 17, 2020 at 11:45:23AM +0900, sy0816.kang@samsung.com wrote: > From: Sunyoung Kang <sy0816.kang@samsung.com> > > get_v4l2_buffer32() didn't copy reserved2 field from userspace to driver. > So the reserved2 value is not received through compat-ioctl32 in driver. > This patch copy reserved2 field of v4l2_buffer in get_v4l2_buffer32(). > > Signed-off-by: Sunyoung Kang <sy0816.kang@samsung.com> > --- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 1 + > 1 file changed, 1 insertion(+) What driver is using the reserved fields? They should be ignored as they are "reserved" for future use. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-17 8:35 ` Greg Kroah-Hartman @ 2020-04-18 3:14 ` Sunyoung Kang 2020-04-18 7:37 ` 'Greg Kroah-Hartman' 0 siblings, 1 reply; 10+ messages in thread From: Sunyoung Kang @ 2020-04-18 3:14 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: mchehab, 'Hans Verkuil', 'Arnd Bergmann', 'Thomas Gleixner', linux-media, linux-kernel Exynos video codec driver uses reserved2 value. How will reserved2 be used for future use? Thanks Sunyoung > -----Original Message----- > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Sent: Friday, April 17, 2020 5:35 PM > To: sy0816.kang@samsung.com > Cc: mchehab@kernel.org; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Arnd > Bergmann <arnd@arndb.de>; Thomas Gleixner <tglx@linutronix.de>; linux- > media@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in > get_v4l2_buffer32 > > On Fri, Apr 17, 2020 at 11:45:23AM +0900, sy0816.kang@samsung.com wrote: > > From: Sunyoung Kang <sy0816.kang@samsung.com> > > > > get_v4l2_buffer32() didn't copy reserved2 field from userspace to driver. > > So the reserved2 value is not received through compat-ioctl32 in driver. > > This patch copy reserved2 field of v4l2_buffer in get_v4l2_buffer32(). > > > > Signed-off-by: Sunyoung Kang <sy0816.kang@samsung.com> > > --- > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 1 + > > 1 file changed, 1 insertion(+) > > What driver is using the reserved fields? They should be ignored as they > are "reserved" for future use. > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-18 3:14 ` Sunyoung Kang @ 2020-04-18 7:37 ` 'Greg Kroah-Hartman' 2020-04-20 0:40 ` Sunyoung Kang 0 siblings, 1 reply; 10+ messages in thread From: 'Greg Kroah-Hartman' @ 2020-04-18 7:37 UTC (permalink / raw) To: Sunyoung Kang Cc: mchehab, 'Hans Verkuil', 'Arnd Bergmann', 'Thomas Gleixner', linux-media, linux-kernel On Sat, Apr 18, 2020 at 12:14:09PM +0900, Sunyoung Kang wrote: > Exynos video codec driver uses reserved2 value. How will reserved2 be used > for future use? No driver should be using the "reserved" fields, as they are "reserved" by the api for future expansion of it. They are not driver-specific fields to be used that way at all. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-18 7:37 ` 'Greg Kroah-Hartman' @ 2020-04-20 0:40 ` Sunyoung Kang 2020-04-20 11:23 ` Arnd Bergmann 0 siblings, 1 reply; 10+ messages in thread From: Sunyoung Kang @ 2020-04-20 0:40 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: mchehab, 'Hans Verkuil', 'Arnd Bergmann', 'Thomas Gleixner', linux-media, linux-kernel I understand what you mean. However, the way to transmit information about the buffer is only flags in v4l2_buffer In flags in v4l2_buffer, there is no reserved bit field that can be used for custom. Additional information about the buffer is needed to provide various functions required by the customers but flags is not enough. So reserved2 is used as an alternative. Can you suggest a better opinion? And copy the reserved2 value in put_v4l2_buffer32(), but it is missing only in get_v4l2_buffer32(). Can't I put it in get_v4l2_buffer32()? Thanks, Sunyoung > -----Original Message----- > From: 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> > Sent: Saturday, April 18, 2020 4:37 PM > To: Sunyoung Kang <sy0816.kang@samsung.com> > Cc: mchehab@kernel.org; 'Hans Verkuil' <hverkuil-cisco@xs4all.nl>; 'Arnd > Bergmann' <arnd@arndb.de>; 'Thomas Gleixner' <tglx@linutronix.de>; linux- > media@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in > get_v4l2_buffer32 > > On Sat, Apr 18, 2020 at 12:14:09PM +0900, Sunyoung Kang wrote: > > Exynos video codec driver uses reserved2 value. How will reserved2 be > > used for future use? > > No driver should be using the "reserved" fields, as they are "reserved" > by the api for future expansion of it. They are not driver-specific > fields to be used that way at all. > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-20 0:40 ` Sunyoung Kang @ 2020-04-20 11:23 ` Arnd Bergmann 2020-04-21 3:33 ` Sunyoung Kang 0 siblings, 1 reply; 10+ messages in thread From: Arnd Bergmann @ 2020-04-20 11:23 UTC (permalink / raw) To: Sunyoung Kang Cc: Greg Kroah-Hartman, Mauro Carvalho Chehab, Hans Verkuil, Thomas Gleixner, Linux Media Mailing List, linux-kernel On Mon, Apr 20, 2020 at 2:40 AM Sunyoung Kang <sy0816.kang@samsung.com> wrote: > > I understand what you mean. > However, the way to transmit information about the buffer is only flags in > v4l2_buffer > In flags in v4l2_buffer, there is no reserved bit field that can be used for > custom. > Additional information about the buffer is needed to provide various > functions required by the customers but flags is not enough. So reserved2 is > used as an alternative. > Can you suggest a better opinion? If you have a driver that needs to pass additional information that is not supported by the subsystem, this is generally either because there is something wrong in the driver, or because there is something wrong in the subsystem. Whichever is at fault should be fixed. If it's the subsystem, then you should explain why it's wrong and make a suggestion for how to address it, e.g. introducing a new ioctl command or redefining the reserved members to be defined in the way you need. In any case, the ioctl commands should be driver independent, so that any hardware with the same feature as your driver can work with the same user space. Arnd ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-20 11:23 ` Arnd Bergmann @ 2020-04-21 3:33 ` Sunyoung Kang 2020-04-21 7:46 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 10+ messages in thread From: Sunyoung Kang @ 2020-04-21 3:33 UTC (permalink / raw) To: 'Arnd Bergmann' Cc: 'Greg Kroah-Hartman', 'Mauro Carvalho Chehab', 'Hans Verkuil', 'Thomas Gleixner', 'Linux Media Mailing List', linux-kernel Thank you for your detailed guide. And I'll look into how to handle the additional information. Thanks Sunyoung > -----Original Message----- > From: Arnd Bergmann <arnd@arndb.de> > Sent: Monday, April 20, 2020 8:23 PM > To: Sunyoung Kang <sy0816.kang@samsung.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Thomas > Gleixner <tglx@linutronix.de>; Linux Media Mailing List <linux- > media@vger.kernel.org>; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in > get_v4l2_buffer32 > > On Mon, Apr 20, 2020 at 2:40 AM Sunyoung Kang <sy0816.kang@samsung.com> > wrote: > > > > I understand what you mean. > > However, the way to transmit information about the buffer is only > > flags in v4l2_buffer In flags in v4l2_buffer, there is no reserved bit > > field that can be used for custom. > > Additional information about the buffer is needed to provide various > > functions required by the customers but flags is not enough. So > > reserved2 is used as an alternative. > > Can you suggest a better opinion? > > If you have a driver that needs to pass additional information that is not > supported by the subsystem, this is generally either because there is > something wrong in the driver, or because there is something wrong in the > subsystem. > > Whichever is at fault should be fixed. If it's the subsystem, then you > should explain why it's wrong and make a suggestion for how to address it, > e.g. > introducing a new ioctl command or redefining the reserved members to be > defined in the way you need. > > In any case, the ioctl commands should be driver independent, so that any > hardware with the same feature as your driver can work with the same user > space. > > Arnd ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 2020-04-21 3:33 ` Sunyoung Kang @ 2020-04-21 7:46 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 10+ messages in thread From: Mauro Carvalho Chehab @ 2020-04-21 7:46 UTC (permalink / raw) To: Sunyoung Kang Cc: 'Arnd Bergmann', 'Greg Kroah-Hartman', 'Hans Verkuil', 'Thomas Gleixner', 'Linux Media Mailing List', linux-kernel Hi Sunyoung, Em Tue, 21 Apr 2020 12:33:42 +0900 "Sunyoung Kang" <sy0816.kang@samsung.com> escreveu: > Thank you for your detailed guide. > And I'll look into how to handle the additional information. Please don't top post. See my comments below. > > Thanks > Sunyoung > > > -----Original Message----- > > From: Arnd Bergmann <arnd@arndb.de> > > Sent: Monday, April 20, 2020 8:23 PM > > To: Sunyoung Kang <sy0816.kang@samsung.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mauro Carvalho Chehab > > <mchehab@kernel.org>; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Thomas > > Gleixner <tglx@linutronix.de>; Linux Media Mailing List <linux- > > media@vger.kernel.org>; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in > > get_v4l2_buffer32 > > > > On Mon, Apr 20, 2020 at 2:40 AM Sunyoung Kang <sy0816.kang@samsung.com> > > wrote: > > > > > > I understand what you mean. > > > However, the way to transmit information about the buffer is only > > > flags in v4l2_buffer In flags in v4l2_buffer, there is no reserved bit > > > field that can be used for custom. > > > Additional information about the buffer is needed to provide various > > > functions required by the customers but flags is not enough. So > > > reserved2 is used as an alternative. > > > Can you suggest a better opinion? > > > > If you have a driver that needs to pass additional information that is not > > supported by the subsystem, this is generally either because there is > > something wrong in the driver, or because there is something wrong in the > > subsystem. > > > > Whichever is at fault should be fixed. If it's the subsystem, then you > > should explain why it's wrong and make a suggestion for how to address it, > > e.g. > > introducing a new ioctl command or redefining the reserved members to be > > defined in the way you need. > > > > In any case, the ioctl commands should be driver independent, so that any > > hardware with the same feature as your driver can work with the same user > > space. I guess the problem here is that the driver that Sunyoung is working is not upstream. The right approach here is to upstream the driver. Once we see the code, we can help addressing the issues. This could either involve using some reserved space at the ioctl for some usage or propose some other solution that would address your needs. This has to be discussed case by case, as it is really hard to say what to do with "additional information that is not supported by the subsystem". What does that exactly means? We need to see the code to better understand it ;-) Thanks, Mauro ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-21 7:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20200417025205epcas2p46d33e64f2de49041d2ca68ecc98fc83e@epcas2p4.samsung.com> 2020-04-17 2:45 ` [PATCH] media: v4l2-compat-ioctl32.c: copy reserved2 field in get_v4l2_buffer32 sy0816.kang 2020-04-17 7:24 ` Mauro Carvalho Chehab 2020-04-18 3:13 ` Sunyoung Kang 2020-04-17 8:35 ` Greg Kroah-Hartman 2020-04-18 3:14 ` Sunyoung Kang 2020-04-18 7:37 ` 'Greg Kroah-Hartman' 2020-04-20 0:40 ` Sunyoung Kang 2020-04-20 11:23 ` Arnd Bergmann 2020-04-21 3:33 ` Sunyoung Kang 2020-04-21 7:46 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).