History log of /linux-master/drivers/misc/bcm-vk/bcm_vk_msg.c
Revision Date Author Comments
# 3c69d52e 22-Sep-2023 Kees Cook <keescook@chromium.org>

misc: bcm-vk: Annotate struct bcm_vk_wkent with __counted_by

Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).

As found with Coccinelle[1], add __counted_by for struct bcm_vk_wkent.
Additionally, since the element count member must be set before accessing
the annotated flexible array member, move its initialization earlier.

[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci

Cc: Scott Branden <scott.branden@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20230922175057.work.558-kees@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 1bae5c0e 29-Jun-2023 Chengfeng Ye <dg573847474@gmail.com>

misc: bcm_vk: Fix potential deadlock on &vk->ctx_lock

As &vk->ctx_lock is acquired by timer bcm_vk_hb_poll() under softirq
context, other process context code should disable irq or bottom-half
before acquire the same lock, otherwise deadlock could happen if the
timer preempt the execution while the lock is held in process context
on the same CPU.

Possible deadlock scenario
bcm_vk_open()
-> bcm_vk_get_ctx()
-> spin_lock(&vk->ctx_lock)
<timer iterrupt>
-> bcm_vk_hb_poll()
-> bcm_vk_blk_drv_access()
-> spin_lock_irqsave(&vk->ctx_lock, flags) (deadlock here)

This flaw was found using an experimental static analysis tool we are
developing for irq-related deadlock, which reported the following
warning when analyzing the linux kernel 6.4-rc7 release.

[Deadlock]: &vk->ctx_lock
[Interrupt]: bcm_vk_hb_poll
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:176
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:512
[Locking Unit]: bcm_vk_ioctl
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:1181
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:512

[Deadlock]: &vk->ctx_lock
[Interrupt]: bcm_vk_hb_poll
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:176
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:512
[Locking Unit]: bcm_vk_ioctl
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:1169

[Deadlock]: &vk->ctx_lock
[Interrupt]: bcm_vk_hb_poll
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:176
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:512
[Locking Unit]: bcm_vk_open
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:216

[Deadlock]: &vk->ctx_lock
[Interrupt]: bcm_vk_hb_poll
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:176
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_dev.c:512
[Locking Unit]: bcm_vk_release
-->/root/linux/drivers/misc/bcm-vk/bcm_vk_msg.c:306

As suggested by Arnd, the tentative patch fix the potential deadlocks
by replacing the timer with delay workqueue. x86_64 allyesconfig using
GCC shows no new warning. Note that no runtime testing was performed
due to no device on hand.

Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
Acked-by: Scott Branden <scott.branden@broadcom.com>
Tested-by: Desmond Yan <desmond.branden@broadcom.com>
Tested-by: Desmond Yan <desmond.yan@broadcom.com>
Link: https://lore.kernel.org/r/20230629182941.13045-1-dg573847474@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 88517757 27-Mar-2022 Jakob Koschel <jakobkoschel@gmail.com>

misc: bcm-vk: replace usage of found with dedicated list iterator variable

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.

To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].

This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Link: https://lore.kernel.org/r/20220327214551.2188544-1-jakobkoschel@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 20827ddd 09-Jun-2021 Baokun Li <libaokun1@huawei.com>

misc: bcm-vk: use list_move_tail instead of list_del/list_add_tail in bcm_vk_msg.c

Using list_move_tail() instead of list_del() + list_add_tail() in bcm_vk_msg.c.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Link: https://lore.kernel.org/r/20210609071430.1337400-1-libaokun1@huawei.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 208012f0 14-May-2021 Gustavo A. R. Silva <gustavoars@kernel.org>

misc: bcm-vk: Replace zero-length array with flexible array member

There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].

Also, make use of the struct_size() helper in kzalloc().

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays

Acked-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20210514230801.GA35863@embeddedor
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# d7a4bfca 01-Feb-2021 Dan Carpenter <dan.carpenter@oracle.com>

misc: bcm-vk: unlock on error in bcm_to_h_msg_dequeue()

Unlock before returning on this error path.

Fixes: 111d746bb476 ("misc: bcm-vk: add VK messaging support")
Acked-by: Desmond Yan <desmond.yan@broadcom.com>
Acked-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/YBfyb+jU5lDUe+5g@mwanda
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# d71277dc 28-Jan-2021 Desmond Yan <desmond.yan@broadcom.com>

misc: bcm-vk: fix set_q_num API precedence issue

Change set_q_num API to use if-else to make it more explicit,
and avoid a precedence rule issue.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Link: https://lore.kernel.org/r/20210129060403.14801-1-scott.branden@broadcom.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# d63d658f 20-Jan-2021 Scott Branden <scott.branden@broadcom.com>

misc: bcm-vk: reset_pid support

Add reset support via ioctl.
Kill user processes that are open when VK card is reset.
If a particular PID has issued the reset request do not kill that process
as it issued the ioctl.

Co-developed-by: Desmond Yan <desmond.yan@broadcom.com>
Acked-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Link: https://lore.kernel.org/r/20210120175827.14820-11-scott.branden@broadcom.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 111d746b 20-Jan-2021 Scott Branden <scott.branden@broadcom.com>

misc: bcm-vk: add VK messaging support

Add message support in order to be able to communicate
to VK card via message queues.

This info is used for debug purposes via collection of logs via direct
read of BAR space and by sysfs access (in a follow on commit).

Co-developed-by: Desmond Yan <desmond.yan@broadcom.com>
Acked-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Link: https://lore.kernel.org/r/20210120175827.14820-10-scott.branden@broadcom.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


# 22c30607 20-Jan-2021 Scott Branden <scott.branden@broadcom.com>

misc: bcm-vk: add open/release

Add open/release to replace private data with context for other methods
to use. Reason for the context is because it is allowed for multiple
sessions to open sysfs. For each file open, when upper layer queries the
response, only those that are tied to a specified open should be returned.

Co-developed-by: Desmond Yan <desmond.yan@broadcom.com>
Acked-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Link: https://lore.kernel.org/r/20210120175827.14820-7-scott.branden@broadcom.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>