| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
My previous bugfix addressed an API inconsistency found by syzbot,
and it correctly fixed the issue on x86-64 machines, which now behave
correctly for both native and compat tasks.
Unfortunately, John found that the patch broke compat mode on all other
architectures, as they can no longer rely on the VIDIOC_DQEVENT_TIME32
code from the native handler as a fallback in the compat code.
The best way I can see for addressing this is to generalize the
VIDIOC_DQEVENT32_TIME32 code from x86 and use that for all architectures,
leaving only the VIDIOC_DQEVENT32 variant as x86 specific. The original
code was trying to be clever and use the same conversion helper for native
32-bit code and compat mode, but that turned out to be too obscure so
even I missed that bit I had introduced myself when I made the fix.
Fixes: c344f07aa1b4 ("media: v4l2-core: ignore native time32 ioctls on 64-bit")
Reported-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
This patch lets user-space request a non-coherent memory
allocation during CREATE_BUFS and REQBUFS ioctl calls.
= CREATE_BUFS
struct v4l2_create_buffers has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= CREATE_BUFS32
struct v4l2_create_buffers32 has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= REQBUFS
We use one byte of a 4 byte ->reserved[1] member of struct
v4l2_requestbuffers. The struct, thus, now has reserved 3 bytes.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
I spotted a minor difference is handling of unregistered devices
between native and compat ioctls: the native handler never tries
to call into the driver if a device is not marked as registered.
I did not check whether this can cause issues in the kernel, or
just a different between return codes, but it clearly makes
sense that both should behave the same way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
In two places time32 structures were defined, but they are not
used if CONFIG_COMPAT_32BIT_TIME is not set. Put these two structs
under #ifdef CONFIG_COMPAT_32BIT_TIME as well to clearly indicate
that they are only used if that config option is set.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
There are no remaining conversions in v4l2_compat_ioctl32(),
so all the infrastructure for it can be removed, with the
only remaining bit being the compat_ioctl32() callback into
drivers that implement their own incompatible data structures.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
There are eight remaining ioctl commands handled by copying
incompatible data structures in v4l2_compat_ioctl32(),
all of them fairly simple.
Change them to instead go through the native ioctl
infrastructure and only special-case the data copy.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
Now that the 'clips' array is accessed by common code in the native
ioctl handler, the same can be done for the compat version, greatly
simplifying the compat code for these four ioctl commands.
[hverkuil: fix: CHECK: Alignment should match open parenthesis]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The ioctl commands based on v4l2_buffer have two sets of compat calls,
one for native time32 structures, and one for compat structures on
64-bit architectures.
Change the compat version to use the same approach as the other simpler
one, for both versions of the structure.
In an earlier version of the patch, I unified the v4l2_buffer_time32
and v4l2_buffer32_time32 compatibility handling into a single
implementation, but that relied on having it all in one file, rather
than having the in_compat_syscall() version in v4l2-compat-ioctl32.c.
[hverkuil: fix various trivial checkpatch issues]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The v4l2_ext_controls ioctl handlers use an indirect pointer to an
incompatible data structure, making the conversion particularly tricky.
Moving the compat implementation to use the new
v4l2_compat_get_user()/v4l2_compat_put_user() helpers makes it
noticeably simpler.
In v4l2_compat_get_array_args()/v4l2_compat_put_array_args(),
the 'file' argument needs to get passed to determine the
exact format, which is a bit unfortunate, as no other conversion
needs these.
[hverkuil: fix: WARNING: Missing a blank line after declarations]
[hverkuil: fix: CHECK: Please don't use multiple blank lines]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
These seven commands are all compatible and do not need any
conversion handlers. The existing ones just copy 32-bit
integers around, and those are always compatible.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The v4l2-compat-ioctl32() currently takes an extra round trip through user
space pointers when converting the data structure formats. In particular,
this involves using the compat_alloc_user_space() and copy_in_user()
helpers that often lead to worse compat handlers compared to using
in_compat_syscall() checks when copying the data.
The native implementation already gained a simpler method to deal with
the conversion for the time32 conversion. Hook into the same places to
provide a location for reading and writing user space data from inside
of the generic video_usercopy() helper.
Hans Verkuil rewrote the video_get_user() function here to simplify
the zeroing of the extra input fields and fixed a couple of bugs in
the original implementation.
[hverkuil: fix: CHECK: Please don't use multiple blank lines]
Co-developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The patch partially reverts some of the UAPI bits of the buffer
cache management hints. Namely, the queue consistency (memory
coherency) user-space hint because, as it turned out, the kernel
implementation of this feature was misusing DMA_ATTR_NON_CONSISTENT.
The patch reverts both kernel and user space parts: removes the
DMA consistency attr functions, rolls back changes to v4l2_requestbuffers,
v4l2_create_buffers structures and corresponding UAPI functions
(plus compat32 layer) and cleans up the documentation.
[hverkuil: fixed a few typos in the commit log]
[hverkuil: fixed vb2_core_reqbufs call in drivers/media/dvb-core/dvb_vb2.c]
[mchehab: fixed a typo in the commit log: revers->reverts]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
This patch lets user-space to request a non-consistent memory
allocation during CREATE_BUFS and REQBUFS ioctl calls.
= CREATE_BUFS
struct v4l2_create_buffers has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= CREATE_BUFS32
struct v4l2_create_buffers32 has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= REQBUFS
We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers,
which is now renamed to ->flags. Unlike v4l2_create_buffers, struct
v4l2_requestbuffers does not have enough reserved room. Therefore for
backward compatibility ->reserved and ->flags were put into anonymous
union.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
Add support for the four new variants of ioctl commands for 64-bit time_t
in v4l2_buffer.
The existing v4l2_buffer32 structure for the traditional format gets
changed to match the new v4l2_buffer format, and the old layout is
now called v4l2_buffer32_time32. Neither of these matches the native
64-bit architecture format of v4l2_buffer, so both require special
handling in compat code.
Duplicating the existing handlers for the new types is a safe conversion
for now, but unfortunately this may turn into a maintenance burden
later. A larger-scale rework of the compat code might be a better
alternative, but is out of scope of the y2038 work.
Note: x32 support for v4l2_buffer32 has always been broken and remains
so after this change, fixing it would have required even more duplication,
and apparently nobody has cared so far.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
[hverkuil-cisco@xs4all.nl: remove spurious newline]
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The native code supports the variant of struct v4l2_event for 64-bit
time_t, so add the compat version as well.
Here, a new incompatibility arises: while almost all 32-bit architectures
now use the same layout as 64-bit architectures and the commands can
simply be passed through, on x86 the internal alignment of v4l2_event
is different because of the 64-bit member in v4l2_event_ctrl.
To handle all architectures, this now requires defining four different
versions of the structure to cover all possible combinations. The compat
handling for VIDIOC_DQEVENT32 and VIDIOC_DQEVENT32_TIME32 is now inside
of an #ifdef so it does not get used on architectures other than x86.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
The do_video_ioctl() compat handler converts the compat command
codes into the native ones before processing further, but this
causes problems for 32-bit user applications that pass a command
code that matches a 64-bit native number, which will then be
handled the same way.
Specifically, this breaks VIDIOC_DQEVENT_TIME from user space
applications with 64-bit time_t, as the structure layout is
the same as the native 64-bit layout on many architectures
(x86 being the notable exception).
Change the handler to use the converted command code only for
passing into the native ioctl handler, not for deciding on the
conversion, in order to make the compat behavior match the
native behavior.
Actual support for the 64-bit time_t version of VIDIOC_DQEVENT_TIME
and other commands still needs to be added in a separate patch.
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
Add SPDX license identifiers to all files which:
- Have no license information of any form
- Have EXPORT_.*_SYMBOL_GPL inside which was used in the
initial scan/conversion to ignore the file
These files fall under the project license, GPL v2 only. The resulting SPDX
license identifier is:
GPL-2.0-only
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Nobody has actually used the type (VERIFY_READ vs VERIFY_WRITE) argument
of the user address range verification function since we got rid of the
old racy i386-only code to walk page tables by hand.
It existed because the original 80386 would not honor the write protect
bit when in kernel mode, so you had to do COW by hand before doing any
user access. But we haven't supported that in a long time, and these
days the 'type' argument is a purely historical artifact.
A discussion about extending 'user_access_begin()' to do the range
checking resulted this patch, because there is no way we're going to
move the old VERIFY_xyz interface to that model. And it's best done at
the end of the merge window when I've done most of my merges, so let's
just get this done once and for all.
This patch was mostly done with a sed-script, with manual fix-ups for
the cases that weren't of the trivial 'access_ok(VERIFY_xyz' form.
There were a couple of notable cases:
- csky still had the old "verify_area()" name as an alias.
- the iter_iov code had magical hardcoded knowledge of the actual
values of VERIFY_{READ,WRITE} (not that they mattered, since nothing
really used it)
- microblaze used the type argument for a debug printout
but other than those oddities this should be a total no-op patch.
I tried to fix up all architectures, did fairly extensive grepping for
access_ok() uses, and the changes are trivial, but I may have missed
something. Any missed conversion should be trivially fixable, though.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
The V4L2_BUF_TYPE_META_OUTPUT mirrors the V4L2_BUF_TYPE_META_CAPTURE with
the exception that it is an OUTPUT type. The use case for this is to pass
buffers to the device that are not image data but metadata. The formats,
just as the metadata capture formats, are typically device specific and
highly structured.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Tested-by: Tian Shu Qiu <tian.shu.qiu@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
Fix this sparse warning:
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:256: warning:
Function parameter or member 'capabilities' not described in 'v4l2_create_buffers32'
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
Set the capabilities field of v4l2_requestbuffers and v4l2_create_buffers.
The various mapping modes were easy, but for signaling the request capability
a new 'supports_requests' bitfield was added to videobuf2-core.h (and set in
vim2m and vivid). Drivers have to set this bitfield for any queue where
requests are supported.
Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
When queuing buffers allow for passing the request that should
be associated with this buffer.
If V4L2_BUF_FLAG_REQUEST_FD is set, then request_fd is used as
the file descriptor.
If a buffer is stored in a request, but not yet queued to the
driver, then V4L2_BUF_FLAG_IN_REQUEST is set.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
If 'which' is V4L2_CTRL_WHICH_REQUEST_VAL, then the 'request_fd' field
can be used to specify a request for the G/S/TRY_EXT_CTRLS ioctls.
Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
This file does a lot of non-trivial struff. Document it using
kernel-doc markups where needed and improve the comments inside
do_video_ioctl().
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Making the cast right for get_user/put_user is not trivial, as
it needs to ensure that the types are the correct ones.
Improve it by using macros.
Tested with vivid with:
$ sudo modprobe vivid no_error_inj=1
$ v4l2-compliance-32bits -a -s10 >32bits && v4l2-compliance-64bits -a -s10 > 64bits && diff -U0 32bits 64bits
--- 32bits 2018-04-17 11:18:29.141240772 -0300
+++ 64bits 2018-04-17 11:18:40.635282341 -0300
@@ -1 +1 @@
-v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 32 bits
+v4l2-compliance SHA : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 bits
Using the latest version of v4l-utils with this patch applied:
https://patchwork.linuxtv.org/patch/48746/
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
In the past, "up" were an acronym for "user pointer" and "kp" for
"kernel pointer". However, since commit a1dfb4c48cc1 ("media:
v4l2-compat-ioctl32.c: refactor compat ioctl32 logic"), both
are now __user pointers.
So, the usage of "kp" is really misleading there. So, rename
both to just "p32" and "p64" everywhere it occurs, in order to
make peace with this file's namespace.
There are two exceptions to "up/kp" nomenclature: at
alloc_userspace() and at do_video_ioctl().
There, a new userspace pointer were allocated, in order to store
the 64 bits version of the ioctl. Those were called as "up_native",
with is, IMHO, an even worse name, as "native" could mislead of
being the arguments that were filled from userspace. I almost
renamed it to just "p64", but, after thinking more about that,
it sounded better to call it as "new_p64", as this makes clearer
that this is the data structure that was allocated inside this
file in order to be used to pass/retrieve data when calling the
64-bit ready file->f_op->unlocked_ioctl() function.
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Smatch report several issues with bad __user annotations:
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: warning: incorrect type in argument 1 (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: expected void [noderef] <asn:1>*uptr
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:447:21: got void *<noident>
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: warning: incorrect type in argument 1 (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: expected void const volatile [noderef] <asn:1>*<noident>
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:621:21: got struct v4l2_plane [noderef] <asn:1>**<noident>
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: warning: incorrect type in argument 1 (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: expected void [noderef] <asn:1>*uptr
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:693:13: got void *[assigned] base
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: warning: incorrect type in assignment (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: expected struct v4l2_ext_control [noderef] <asn:1>*kcontrols
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:871:13: got struct v4l2_ext_control *<noident>
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: warning: incorrect type in assignment (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: expected unsigned char [usertype] *__pu_val
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:957:13: got void [noderef] <asn:1>*
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: warning: incorrect type in argument 1 (different address spaces)
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: expected void [noderef] <asn:1>*uptr
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:973:13: got void *[assigned] edid
Fix them.
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
As warned by smatch:
drivers/media/v4l2-core/v4l2-compat-ioctl32.c:879 put_v4l2_ext_controls32() warn: check for integer overflow 'count'
The access_ok() logic should check for too big arrays too.
Cc: stable@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
At put_v4l2_window32(), it tries to access kp->clips. However,
kp points to an userspace pointer. So, it should be obtained
via get_user(), otherwise it can OOPS:
vivid-000: ================== END STATUS ==================
BUG: unable to handle kernel paging request at 00000000fffb18e0
IP: [<ffffffffc05468d9>] __put_v4l2_format32+0x169/0x220 [videodev]
PGD 3f5776067 PUD 3f576f067 PMD 3f5769067 PTE 800000042548f067
Oops: 0001 [#1] SMP
Modules linked in: vivid videobuf2_vmalloc videobuf2_memops v4l2_dv_timings videobuf2_core v4l2_common videodev media xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables bluetooth rfkill binfmt_misc snd_hda_codec_hdmi i915 snd_hda_intel snd_hda_controller snd_hda_codec intel_rapl x86_pkg_temp_thermal snd_hwdep intel_powerclamp snd_pcm coretemp snd_seq_midi kvm_intel kvm snd_seq_midi_event snd_rawmidi i2c_algo_bit drm_kms_helper snd_seq drm crct10dif_pclmul e1000e snd_seq_device crc32_pclmul snd_timer ghash_clmulni_intel snd mei_me mei ptp pps_core soundcore lpc_ich video crc32c_intel [last unloaded: media]
CPU: 2 PID: 28332 Comm: v4l2-compliance Not tainted 3.18.102+ #107
Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
task: ffff8804293f8000 ti: ffff8803f5640000 task.ti: ffff8803f5640000
RIP: 0010:[<ffffffffc05468d9>] [<ffffffffc05468d9>] __put_v4l2_format32+0x169/0x220 [videodev]
RSP: 0018:ffff8803f5643e28 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000fffb1ab4
RDX: 00000000fffb1a68 RSI: 00000000fffb18d8 RDI: 00000000fffb1aa8
RBP: ffff8803f5643e48 R08: 0000000000000001 R09: ffff8803f54b0378
R10: 0000000000000000 R11: 0000000000000168 R12: 00000000fffb18c0
R13: 00000000fffb1a94 R14: 00000000fffb18c8 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff880456d00000(0063) knlGS:00000000f7100980
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00000000fffb18e0 CR3: 00000003f552b000 CR4: 00000000003407e0
Stack:
00000000fffb1a94 00000000c0cc5640 0000000000000056 ffff8804274f3600
ffff8803f5643ed0 ffffffffc0547e16 0000000000000003 ffff8803f5643eb0
ffffffff81301460 ffff88009db44b01 ffff880441942520 ffff8800c0d05640
Call Trace:
[<ffffffffc0547e16>] v4l2_compat_ioctl32+0x12d6/0x1b1d [videodev]
[<ffffffff81301460>] ? file_has_perm+0x70/0xc0
[<ffffffff81252a2c>] compat_SyS_ioctl+0xec/0x1200
[<ffffffff8173241a>] sysenter_dispatch+0x7/0x21
Code: 00 00 48 8b 80 48 c0 ff ff 48 83 e8 38 49 39 c6 0f 87 2b ff ff ff 49 8d 45 1c e8 a3 ce e3 c0 85 c0 0f 85 1a ff ff ff 41 8d 40 ff <4d> 8b 64 24 20 41 89 d5 48 8d 44 40 03 4d 8d 34 c4 eb 15 0f 1f
RIP [<ffffffffc05468d9>] __put_v4l2_format32+0x169/0x220 [videodev]
RSP <ffff8803f5643e28>
CR2: 00000000fffb18e0
Tested with vivid driver on Kernel v3.18.102.
Same bug happens upstream too:
BUG: KASAN: user-memory-access in __put_v4l2_format32+0x98/0x4d0 [videodev]
Read of size 8 at addr 00000000ffe48400 by task v4l2-compliance/8713
CPU: 0 PID: 8713 Comm: v4l2-compliance Not tainted 4.16.0-rc4+ #108
Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
Call Trace:
dump_stack+0x5c/0x7c
kasan_report+0x164/0x380
? __put_v4l2_format32+0x98/0x4d0 [videodev]
__put_v4l2_format32+0x98/0x4d0 [videodev]
v4l2_compat_ioctl32+0x1aec/0x27a0 [videodev]
? __fsnotify_inode_delete+0x20/0x20
? __put_v4l2_format32+0x4d0/0x4d0 [videodev]
compat_SyS_ioctl+0x646/0x14d0
? do_ioctl+0x30/0x30
do_fast_syscall_32+0x191/0x3f4
entry_SYSENTER_compat+0x6b/0x7a
==================================================================
Disabling lock debugging due to kernel taint
BUG: unable to handle kernel paging request at 00000000ffe48400
IP: __put_v4l2_format32+0x98/0x4d0 [videodev]
PGD 3a22fb067 P4D 3a22fb067 PUD 39b6f0067 PMD 39b6f1067 PTE 80000003256af067
Oops: 0001 [#1] SMP KASAN
Modules linked in: vivid videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops v4l2_tpg v4l2_dv_timings videobuf2_v4l2 videobuf2_common v4l2_common videodev xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack libcrc32c tun bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic binfmt_misc snd_hda_codec_hdmi intel_rapl x86_pkg_temp_thermal intel_powerclamp i915 coretemp snd_hda_intel snd_hda_codec kvm_intel snd_hwdep snd_hda_core kvm snd_pcm irqbypass crct10dif_pclmul crc32_pclmul snd_seq_midi ghash_clmulni_intel snd_seq_midi_event i2c_algo_bit intel_cstate snd_rawmidi intel_uncore snd_seq drm_kms_helper e1000e snd_seq_device snd_timer intel_rapl_perf
drm ptp snd mei_me mei lpc_ich pps_core soundcore video crc32c_intel
CPU: 0 PID: 8713 Comm: v4l2-compliance Tainted: G B 4.16.0-rc4+ #108
Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
RIP: 0010:__put_v4l2_format32+0x98/0x4d0 [videodev]
RSP: 0018:ffff8803b9be7d30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8803ac983e80 RCX: ffffffff8cd929f2
RDX: 1ffffffff1d0a149 RSI: 0000000000000297 RDI: 0000000000000297
RBP: 00000000ffe485c0 R08: fffffbfff1cf5123 R09: ffffffff8e7a8948
R10: 0000000000000001 R11: fffffbfff1cf5122 R12: 00000000ffe483e0
R13: 00000000ffe485c4 R14: ffff8803ac985918 R15: 00000000ffe483e8
FS: 0000000000000000(0000) GS:ffff880407400000(0063) knlGS:00000000f7a46980
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00000000ffe48400 CR3: 00000003a83f2003 CR4: 00000000003606f0
Call Trace:
v4l2_compat_ioctl32+0x1aec/0x27a0 [videodev]
? __fsnotify_inode_delete+0x20/0x20
? __put_v4l2_format32+0x4d0/0x4d0 [videodev]
compat_SyS_ioctl+0x646/0x14d0
? do_ioctl+0x30/0x30
do_fast_syscall_32+0x191/0x3f4
entry_SYSENTER_compat+0x6b/0x7a
Code: 4c 89 f7 4d 8d 7c 24 08 e8 e6 a4 69 cb 48 8b 83 98 1a 00 00 48 83 e8 10 49 39 c7 0f 87 9d 01 00 00 49 8d 7c 24 20 e8 c8 a4 69 cb <4d> 8b 74 24 20 4c 89 ef 4c 89 fe ba 10 00 00 00 e8 23 d9 08 cc
RIP: __put_v4l2_format32+0x98/0x4d0 [videodev] RSP: ffff8803b9be7d30
CR2: 00000000ffe48400
cc: stable@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
If the device is of type VFL_TYPE_SUBDEV then vdev->ioctl_ops
is NULL so the 'if (!ops->vidioc_query_ext_ctrl)' check would crash.
Add a test for !ops to the condition.
All sub-devices that have controls will use the control framework,
so they do not have an equivalent to ops->vidioc_query_ext_ctrl.
Returning false if ops is NULL is the correct thing to do here.
Fixes: b8c601e8af ("v4l2-compat-ioctl32.c: fix ctrl_is_pointer")
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The 32-bit compat v4l2 ioctl handling is implemented based on its 64-bit
equivalent. It converts 32-bit data structures into its 64-bit
equivalents and needs to provide the data to the 64-bit ioctl in user
space memory which is commonly allocated using
compat_alloc_user_space().
However, due to how that function is implemented, it can only be called
a single time for every syscall invocation.
Supposedly to avoid this limitation, the existing code uses a mix of
memory from the kernel stack and memory allocated through
compat_alloc_user_space().
Under normal circumstances, this would not work, because the 64-bit
ioctl expects all pointers to point to user space memory. As a
workaround, set_fs(KERNEL_DS) is called to temporarily disable this
extra safety check and allow kernel pointers. However, this might
introduce a security vulnerability: The result of the 32-bit to 64-bit
conversion is writeable by user space because the output buffer has been
allocated via compat_alloc_user_space(). A malicious user space process
could then manipulate pointers inside this output buffer, and due to the
previous set_fs(KERNEL_DS) call, functions like get_user() or put_user()
no longer prevent kernel memory access.
The new approach is to pre-calculate the total amount of user space
memory that is needed, allocate it using compat_alloc_user_space() and
then divide up the allocated memory to accommodate all data structures
that need to be converted.
An alternative approach would have been to retain the union type karg
that they allocated on the kernel stack in do_video_ioctl(), copy all
data from user space into karg and then back to user space. However, we
decided against this approach because it does not align with other
compat syscall implementations. Instead, we tried to replicate the
get_user/put_user pairs as found in other places in the kernel:
if (get_user(clipcount, &up->clipcount) ||
put_user(clipcount, &kp->clipcount)) return -EFAULT;
Notes from hans.verkuil@cisco.com:
This patch was taken from:
https://github.com/LineageOS/android_kernel_samsung_apq8084/commit/97b733953c06e4f0398ade18850f0817778255f7
Clearly nobody could be bothered to upstream this patch or at minimum
tell us :-( We only heard about this a week ago.
This patch was rebased and cleaned up. Compared to the original I
also swapped the order of the convert_in_user arguments so that they
matched copy_in_user. It was hard to review otherwise. I also replaced
the ALLOC_USER_SPACE/ALLOC_AND_GET by a normal function.
Fixes: 6b5a9492ca ("v4l: introduce string control support.")
Signed-off-by: Daniel Mentz <danielmentz@google.com>
Co-developed-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Some ioctls need to copy back the result even if the ioctl returned
an error. However, don't do this for the error code -ENOTTY.
It makes no sense in that cases.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
There is nothing wrong with using an unknown buffer type. So
stop spamming the kernel log whenever this happens. The kernel
will just return -EINVAL to signal this.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
put_v4l2_window32() didn't copy back the clip list to userspace.
Drivers can update the clip rectangles, so this should be done.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
ctrl_is_pointer just hardcoded two known string controls, but that
caused problems when using e.g. custom controls that use a pointer
for the payload.
Reimplement this function: it now finds the v4l2_ctrl (if the driver
uses the control framework) or it calls vidioc_query_ext_ctrl (if the
driver implements that directly).
In both cases it can now check if the control is a pointer control
or not.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The struct v4l2_plane32 should set m.userptr as well. The same
happens in v4l2_buffer32 and v4l2-compliance tests for this.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Instead of doing sizeof(struct foo) use sizeof(*up). There even were
cases where 4 * sizeof(__u32) was used instead of sizeof(kp->reserved),
which is very dangerous when the size of the reserved array changes.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
These helper functions do not really help. Move the code to the
__get/put_v4l2_format32 functions.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The indentation of this source is all over the place. Fix this.
This patch only changes whitespace.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The result of the VIDIOC_PREPARE_BUF ioctl was never copied back
to userspace since it was missing in the switch.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: <stable@vger.kernel.org> # for v4.15 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
There are a lot of places where sequences of space/tabs are
found. Get rid of all spaces before tabs.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
On several places, whitespaces are being used for indentation,
or even at the end of the line.
Fix them.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The v4l2_input32 struct wasn't updated when this field was added.
It didn't cause a failure in the compat code, but it is better to
keep it in sync with v4l2_input to avoid confusion.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Certain syscalls like recvmmsg support 64 bit timespec values for the
X32 ABI. The helper function compat_put_timespec converts a timespec
value to a 32 bit or 64 bit value depending on what ABI is used. The
v4l2 compat layer, however, is not designed to support 64 bit timespec
values and always uses 32 bit values. Hence, compat_put_timespec must
not be used.
Without this patch, user space will be provided with bad timestamp
values from the VIDIOC_DQEVENT ioctl. Also, fields of the struct
v4l2_event32 that come immediately after timestamp get overwritten,
namely the field named id.
Fixes: 81993e81a994 ("compat: Get rid of (get|put)_compat_time(val|spec)")
Cc: stable@vger.kernel.org
Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Tiffany Lin <tiffany.lin@mediatek.com>
Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Daniel Mentz <danielmentz@google.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Commit b2787845fb91 ("V4L/DVB (5289): Add support for video output
overlays.") added the field global_alpha to struct v4l2_window but did
not update the compat layer accordingly. This change adds global_alpha
to struct v4l2_window32 and copies the value for global_alpha back and
forth.
Signed-off-by: Daniel Mentz <danielmentz@google.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
The metadata buffer type is used to transfer metadata between userspace
and kernelspace through a V4L2 buffers queue. It comes with a new
metadata capture capability and format description.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Tested-by: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
[hans.verkuil@cisco.com: removed left-over 'experimental' note]
[hans.verkuil@cisco.com: add newline after _v4l2-meta-format label]
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Most ioctls do not have to write back the contents of the argument
if an error is returned. But VIDIOC_S_EDID is an exception together
with the EXT_CTRLS ioctls (already handled correctly).
Add this exception to v4l2-compat-ioctl32.
This fixes a compliance error when using compat32 and trying to
set a new EDID with more blocks than the hardware supports. In
that case the driver will return -E2BIG and set edid.blocks to the
actual maximum number of blocks. This field was never copied back
to userspace due to this bug.
Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
get_v4l2_ext_controls32() is used to convert the 32-bit compat struct into
native 64-bit representation. The function multiplies the array length by
the entry length before validating size. Perform the size validation
first.
Also use unsigned values for size computation.
Make similar changes to get_v4l2_buffer32() for multi-plane buffers.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
put_v4l2_create32
In v4l2-compliance utility, test VIDIOC_CREATE_BUFS will check whether reserved
filed of v4l2_create_buffers filled with zero
Reserved field is filled with zero in v4l_create_bufs.
This patch copy reserved field of v4l2_create_buffer from kernel space to user
space
Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: <stable@vger.kernel.org> # for v3.19 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
|
|
In v4l2-compliance utility, test QUERYBUF required correct length
value to go through each planar to check planar's length in
multi-planar buffer type
Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: <stable@vger.kernel.org> # for v3.7 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
|
