| Age | Commit message (Collapse) | Author |
|---|
|
When the first test device was added (vivi.c), there were just
one file. I was too lazy on that time to create a separate
directory just for it, so I kept it together with platform.
Now, we have vivid, vicodec, vim2m and vimc. Also, a new
virtual driver has been prepared to support DVB API.
So, it is time to solve this mess, by placing test stuff
on a separate directory.
It should be noticed that we also have some skeleton drivers
(for V4L and for DVB). For now, we'll keep them separate,
as they're not really test drivers, but instead, just
examples. The DVB frontend ones will likely be part of a new DVB
test driver. By that time, it should make sense to move them
here as well.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
A use-after-free bug occures when unbinding the device while it streams.
The 'struct vimc_ent_device' allocated for the 'Sensor A' is freed
when calling the sensor's 'rm' callback but the freed pointer is
later accessed in the function 'vimc_streamer_pipeline_terminate'.
To fix this bug, move the release callback of the vimc entities
and vimc_device to the release callback of v4l2_device.
The .rm callback of vimc_ent_config is replaced by two callbacks:
.unregister - this is called upon removing the device and
it unregisters the entity. This is an optional callback since
subdevices don't need to implement it because they are already
unregistered in v4l2_device_unregister.
.release - this is called from the release callback of v4l2_device
and it frees the entity.
This ensures that the entities will be released when the last fh
of any of the devices is closed.
The commands that cause the crash and the KASAN report:
media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
v4l2-ctl --stream-mmap --stream-count=1000 -d /dev/video2 &
sleep 1
echo -n vimc.0 >/sys/bus/platform/drivers/vimc/unbind
[ 188.417934] BUG: KASAN: use-after-free in vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.420182] Read of size 8 at addr ffff8881e9c26008 by task bash/185
[ 188.421800]
[ 188.422223] CPU: 0 PID: 185 Comm: bash Not tainted 5.5.0-rc1+ #1
[ 188.423681] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 188.425938] Call Trace:
[ 188.426610] dump_stack+0x75/0xa0
[ 188.427519] ? vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.429057] print_address_description.constprop.6+0x16/0x220
[ 188.430462] ? vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.431979] ? vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.433455] __kasan_report.cold.9+0x1a/0x40
[ 188.434518] ? vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.436010] kasan_report+0xe/0x20
[ 188.436859] vimc_streamer_pipeline_terminate+0x75/0x140 [vimc]
[ 188.438339] vimc_streamer_s_stream+0x8b/0x3c0 [vimc]
[ 188.439576] vimc_cap_stop_streaming+0x22/0x40 [vimc]
[ 188.440863] __vb2_queue_cancel+0x65/0x560 [videobuf2_common]
[ 188.442391] vb2_core_queue_release+0x19/0x50 [videobuf2_common]
[ 188.443974] vimc_cap_rm+0x10/0x20 [vimc]
[ 188.444986] vimc_rm_subdevs+0x9e/0xe0 [vimc]
[ 188.446179] vimc_remove+0x19/0x70 [vimc]
[ 188.447301] platform_drv_remove+0x2f/0x50
[ 188.448468] device_release_driver_internal+0x133/0x260
[ 188.449814] unbind_store+0x121/0x150
[ 188.450726] kernfs_fop_write+0x142/0x230
[ 188.451724] ? sysfs_kf_bin_read+0x100/0x100
[ 188.452826] vfs_write+0xdc/0x230
[ 188.453760] ksys_write+0xaf/0x140
[ 188.454702] ? __ia32_sys_read+0x40/0x40
[ 188.455773] ? __do_page_fault+0x473/0x620
[ 188.456780] do_syscall_64+0x5e/0x1a0
[ 188.457711] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 188.459079] RIP: 0033:0x7f80f1f13504
[ 188.459969] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 48 8d 05 f9 61 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53
[ 188.464445] RSP: 002b:00007ffd7e843b58 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 188.466276] RAX: ffffffffffffffda RBX: 0000000000000006 RCX: 00007f80f1f13504
[ 188.467999] RDX: 0000000000000006 RSI: 000055ef2eb21b10 RDI: 0000000000000001
[ 188.469708] RBP: 000055ef2eb21b10 R08: 00007f80f1fe68c0 R09: 00007f80f1e26740
[ 188.471407] R10: 000055ef2eade010 R11: 0000000000000246 R12: 00007f80f1fe5760
[ 188.473381] R13: 0000000000000006 R14: 00007f80f1fe0760 R15: 0000000000000006
[ 188.475107]
[ 188.475500] Allocated by task 473:
[ 188.476351] save_stack+0x19/0x80
[ 188.477201] __kasan_kmalloc.constprop.6+0xc1/0xd0
[ 188.478507] vimc_sen_add+0x36/0x309 [vimc]
[ 188.479649] vimc_probe+0x1e2/0x530 [vimc]
[ 188.480776] platform_drv_probe+0x46/0xa0
[ 188.481829] really_probe+0x16c/0x520
[ 188.482732] driver_probe_device+0x114/0x170
[ 188.483783] device_driver_attach+0x85/0x90
[ 188.484800] __driver_attach+0xa8/0x190
[ 188.485734] bus_for_each_dev+0xe4/0x140
[ 188.486702] bus_add_driver+0x223/0x2d0
[ 188.487715] driver_register+0xca/0x140
[ 188.488767] 0xffffffffc037003d
[ 188.489635] do_one_initcall+0x86/0x28f
[ 188.490702] do_init_module+0xf8/0x340
[ 188.491773] load_module+0x3766/0x3a10
[ 188.492811] __do_sys_finit_module+0x11a/0x1b0
[ 188.494059] do_syscall_64+0x5e/0x1a0
[ 188.495079] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 188.496481]
[ 188.496893] Freed by task 185:
[ 188.497670] save_stack+0x19/0x80
[ 188.498493] __kasan_slab_free+0x125/0x170
[ 188.499486] kfree+0x8c/0x230
[ 188.500254] v4l2_subdev_release+0x64/0x70 [videodev]
[ 188.501498] v4l2_device_release_subdev_node+0x1c/0x30 [videodev]
[ 188.502976] device_release+0x3c/0xd0
[ 188.503867] kobject_put+0xf4/0x240
[ 188.507802] vimc_rm_subdevs+0x9e/0xe0 [vimc]
[ 188.508846] vimc_remove+0x19/0x70 [vimc]
[ 188.509792] platform_drv_remove+0x2f/0x50
[ 188.510752] device_release_driver_internal+0x133/0x260
[ 188.512006] unbind_store+0x121/0x150
[ 188.512899] kernfs_fop_write+0x142/0x230
[ 188.513874] vfs_write+0xdc/0x230
[ 188.514698] ksys_write+0xaf/0x140
[ 188.515523] do_syscall_64+0x5e/0x1a0
[ 188.516543] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 188.517710]
[ 188.518034] The buggy address belongs to the object at ffff8881e9c26000
[ 188.518034] which belongs to the cache kmalloc-4k of size 4096
[ 188.520528] The buggy address is located 8 bytes inside of
[ 188.520528] 4096-byte region [ffff8881e9c26000, ffff8881e9c27000)
[ 188.523015] The buggy address belongs to the page:
[ 188.524357] page:ffffea0007a70800 refcount:1 mapcount:0 mapping:ffff8881f6402140 index:0x0 compound_mapcount: 0
[ 188.527058] raw: 0200000000010200 dead000000000100 dead000000000122 ffff8881f6402140
[ 188.528983] raw: 0000000000000000 0000000000040004 00000001ffffffff 0000000000000000
[ 188.530883] page dumped because: kasan: bad access detected
[ 188.532336]
[ 188.532720] Memory state around the buggy address:
[ 188.533871] ffff8881e9c25f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 188.535631] ffff8881e9c25f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 188.537370] >ffff8881e9c26000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 188.538996] ^
[ 188.539812] ffff8881e9c26080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 188.541549] ffff8881e9c26100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
In future patch, the release of the device will move
to the release callback of v4l2_device. Therefore the
device will be released only when the last fh will be
closed. Dynamic allocation will then be needed since
when the device is unbounded and then bounded again,
it might be that the probe callback will run before
the release of the last device is finished. In that
case both operations will run on the same memory
concurrently and cause memory corruption.
This patch also removes the pdev field of
vimc_device since it is not needed anymore.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
replace 'vimc->pdev.dev' with 'vimc->mdev.dev'
in debug prints and in assignment to
vimc_ent_device.dev. This helps to unify the debug
statements. This will also eliminate the need to use
the pdev field in vimc_device in future patch.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
|
|
Since the add callback returns NULL on failure and the array
is initialized to NULLs, there is no need for the intermediate
assignment to local var.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
since NULL value for vimc entity pointer indicates
that entity creation failed and this is tested, the
pointers should be initialized to NULL.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Fixes: f13d5f361959 ("media: vimc: Collapse component structure into a single monolithic driver")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
vimc uses Component API to split the driver into functional components.
The real hardware resembles a monolith structure than component and
component structure added a level of complexity making it hard to
maintain without adding any real benefit.
The sensor is one vimc component that would makes sense to be a separate
module to closely align with the real hardware. It would be easier to
collapse vimc into single monolithic driver first and then split the
sensor off as a separate module.
Collapse it into a single monolithic driver removing the Component API.
This patch removes the component API and makes minimal changes to the
code base preserving the functional division of the code structure.
Preserving the functional structure allows us to split the sensor off
as a separate module in the future.
Major design elements in this change are:
- Use existing struct vimc_ent_config and struct vimc_pipeline_config
to drive the initialization of the functional components.
- Make vimc_device and vimc_ent_config global by moving them to
vimc-common.h
- Add two new hooks add and rm to initialize and register, unregister
and free subdevs.
- All component API is now gone and bind and unbind hooks are modified
to do "add" and "rm" with minimal changes to just add and rm subdevs.
- vimc-core's bind and unbind are now register and unregister.
- Add a new field to vimc_device structure for saving the pointers to
struct vimc_ent_device(s) subdevs create in their "add" hooks. These
get used to create links and removing the subdevs. vimc-core allocates
this array which sized to number of entries in the topology defined in
the vimc_pipeline_config structure.
- vimc-core invokes "add" hooks from its vimc_register_devices().
The "add" hooks remain the same and register subdevs. They don't
create platform devices of their own and use vimc's pdev.dev as
their reference device. Each "add" hook returns pointer to its struct
vimc_ent_device. This is saved in the vimc_device ent_devs array.
- vimc-core invokes "rm" hooks from its unregister to unregister subdevs
and cleanup.
- vimc-core invokes "add" and "rm" hooks with pointer to struct vimc_device
and the corresponding vimc_ent_device saved in the ent_devs.
The following configure and stream test works on all devices.
media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video1
v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2
v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video3
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
There are some conflicts due to SPDX changes. We also have more
patches being merged via media tree touching them.
So, let's merge back from upstream and address those.
Linux 5.2-rc4
* tag 'v5.2-rc4': (767 commits)
Linux 5.2-rc4
MAINTAINERS: Karthikeyan Ramasubramanian is MIA
i2c: xiic: Add max_read_len quirk
lockref: Limit number of cmpxchg loop retries
uaccess: add noop untagged_addr definition
x86/insn-eval: Fix use-after-free access to LDT entry
kbuild: use more portable 'command -v' for cc-cross-prefix
s390/unwind: correct stack switching during unwind
block, bfq: add weight symlink to the bfq.weight cgroup parameter
cgroup: let a symlink too be created with a cftype file
drm/nouveau/secboot/gp10[2467]: support newer FW to fix SEC2 failures on some boards
drm/nouveau/secboot: enable loading of versioned LS PMU/SEC2 ACR msgqueue FW
drm/nouveau/secboot: split out FW version-specific LS function pointers
drm/nouveau/secboot: pass max supported FW version to LS load funcs
drm/nouveau/core: support versioned firmware loading
drm/nouveau/core: pass subdev into nvkm_firmware_get, rather than device
block: free sched's request pool in blk_cleanup_queue
pktgen: do not sleep with the thread lock held.
net: mvpp2: Use strscpy to handle stat strings
net: rds: fix memory leak in rds_ib_flush_mr_pool
...
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
Based on 3 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [kishon] [vijay] [abraham]
[i] [kishon]@[ti] [com] this program is distributed in the hope that
it will be useful but without any warranty without even the implied
warranty of merchantability or fitness for a particular purpose see
the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [graeme] [gregory]
[gg]@[slimlogic] [co] [uk] [author] [kishon] [vijay] [abraham] [i]
[kishon]@[ti] [com] [based] [on] [twl6030]_[usb] [c] [author] [hema]
[hk] [hemahk]@[ti] [com] this program is distributed in the hope
that it will be useful but without any warranty without even the
implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1105 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.202006027@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
If the system has other devices being registered in the component
framework, the compare function will be called with a device that
doesn't belong to vimc.
This device is not necessarily a platform_device, nor have a
platform_data (which causes a NULL pointer dereference error) and if it
does have a pdata, it is not necessarily type of struct vimc_platform_data.
So casting to any of these types is wrong.
Instead of expecting a given pdev with a given pdata, just expect for
the device it self. vimc-core is the one who creates them, we know in
advance exactly which object to expect in the match.
Fixes: 4a29b7090749 ("[media] vimc: Subdevices as modules")
Signed-off-by: Helen Koike <helen.koike@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
The media_device is part of a static global vimc_device struct.
The media framework expects this to be zeroed before it is
used, however, since this is a global this is not the case if
vimc is unbound and then bound again.
So call memset to ensure any left-over values are cleared.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
media_device_cleanup() wasn't called, which caused a small
memory leak.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
It is good practice to fill in bus_info.
Also just use 'platform:vimc' when filling in the bus_info in querycap:
the bus_info has nothing to do with the video device name.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
The driver name as returned in v4l2_capabilities must be vimc, not vimc_capture.
Fix this.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
The implementation of strscpy() is more robust and safer.
That's now the recommended way to copy NUL terminated strings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
Commit 4a29b7090749 ("[media] vimc: Subdevices as modules") removes
vimc allocation from vimc_probe(), so corresponding deallocation
on the error path tries to free static memory.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
|
|
In case of error, the function platform_device_register_data() returns
ERR_PTR() and never returns NULL. The NULL test in the return value check
should be replaced with IS_ERR().
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
|
|
Change the core structure for adding subdevices in the topology.
Instead of calling the specific create function for each subdevice,
inject a child platform_device with the driver's name.
Each type of node in the topology (sensor, capture, debayer, scaler)
will register a platform_driver with the corresponding name through the
component subsystem.
Implementing a new subdevice type doesn't require vimc-core to be altered.
This facilitates future implementation of dynamic entities, where
hotpluging an entity in the topology is just a matter of
registering/unregistering a platform_device in the system.
It also facilitates other implementations of different nodes without
touching the core code and remove the need of a header file for each
type of node.
Signed-off-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
Remove helper functions from vimc-core and add it in vimc-common to
clean up the core.
Signed-off-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
|
First version of the Virtual Media Controller.
Add a simple version of the core of the driver, the capture and
sensor nodes in the topology, generating a grey image in a hardcoded
format.
Signed-off-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[hans.verkuil@cisco.com: fix small typo in Kconfig]
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
|
