greybus-dev

greybus-dev@lists.linaro.org

6 participants
859 discussions

[PATCH] greybus: es2: drop redundant device reference

by Johan Hovold

Driver core holds a reference to the USB interface and its parent USB device while the interface is bound to a driver and there is no need to take additional references unless the structures are needed after disconnect. Drop the redundant device reference to reduce cargo culting, make it easier to spot drivers where an extra reference is needed, and reduce the risk of memory leaks when drivers fail to release it. Signed-off-by: Johan Hovold <johan(a)kernel.org> --- drivers/greybus/es2.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/greybus/es2.c b/drivers/greybus/es2.c index 6ae0ac828afa..bb513ef7f7ff 100644 --- a/drivers/greybus/es2.c +++ b/drivers/greybus/es2.c @@ -806,8 +806,6 @@ static void es2_destroy(struct es2_ap_dev *es2) udev = es2->usb_dev; gb_hd_put(es2->hd); - - usb_put_dev(udev); } static void cport_in_callback(struct urb *urb) @@ -1257,7 +1255,7 @@ static int ap_probe(struct usb_interface *interface, bool bulk_in_found = false; bool arpc_in_found = false; - udev = usb_get_dev(interface_to_usbdev(interface)); + udev = interface_to_usbdev(interface); num_cports = apb_get_cport_count(udev); if (num_cports < 0) { -- 2.52.0

6 months

[PATCH] staging: greybus: remove redundant 'int' from unsigned long long

by Oskar Ray-Frayssinet

Replace 'unsigned long long int' with 'unsigned long long' as the 'int' suffix is unnecessary and not preferred by kernel coding style. Signed-off-by: Oskar Ray-Frayssinet <rayfraytech(a)gmail.com> --- drivers/staging/greybus/Documentation/firmware/authenticate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c index 3d2c6f88a138..0ef88b7d24de 100644 --- a/drivers/staging/greybus/Documentation/firmware/authenticate.c +++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c @@ -58,7 +58,7 @@ int main(int argc, char *argv[]) goto close_fd; } - printf("UID received: 0x%llx\n", *(unsigned long long int *)(uid.uid)); + printf("UID received: 0x%llx\n", *(unsigned long long *)(uid.uid)); /* Get certificate */ printf("Get IMS certificate\n"); -- 2.43.0

6 months

[PATCH 1/2] greybus: raw: fix use-after-free on cdev close

by Damien Riégel

This addresses a use-after-free bug when a raw bundle is disconnected but its chardev is still opened by an application. When the application releases the cdev, it causes the following panic when init on free is enabled (CONFIG_INIT_ON_FREE_DEFAULT_ON=y): [ 78.451062] refcount_t: underflow; use-after-free. [ 78.451352] WARNING: CPU: 0 PID: 139 at lib/refcount.c:28 refcount_warn_saturate+0xd0/0x130 [ 78.451698] Modules linked in: gb_raw(C) [ 78.451881] CPU: 0 UID: 0 PID: 139 Comm: raw_chardev_tes Tainted: G WC 6.18.0-rc4 #212 PREEMPT(voluntary) [ 78.452386] Tainted: [W]=WARN, [C]=CRAP [ 78.452560] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014 [ 78.453049] RIP: 0010:refcount_warn_saturate+0xd0/0x130 [ 78.453311] Code: 0b 90 90 c3 cc cc cc cc 80 3d 4f ec 1d 01 00 0f 85 75 ff ff ff c6 05 42 ec 1d 01 01 90 48 c7 c7 e8 5b cb b4 e8 31f [ 78.453953] RSP: 0018:ffffaa0f80203ed0 EFLAGS: 00010282 [ 78.454251] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 78.454472] RDX: 0000000000000000 RSI: ffffaa0f80203d68 RDI: 00000000ffffdfff [ 78.454690] RBP: 00000000040e001f R08: 00000000ffffdfff R09: ffffffffb510c008 [ 78.454899] R10: ffffffffb505c060 R11: 0000000063666572 R12: ffff938dc210b468 [ 78.455279] R13: ffff938dc1f5e1a0 R14: ffff938dc14710c0 R15: 0000000000000000 [ 78.455549] FS: 00007f2f22741740(0000) GS:ffff938e11fbc000(0000) knlGS:0000000000000000 [ 78.455806] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 78.456129] CR2: 00007f2f228c89c3 CR3: 00000000020d0000 CR4: 00000000000006f0 [ 78.456786] Call Trace: [ 78.456936] <TASK> [ 78.457069] cdev_put+0x18/0x30 [ 78.457230] __fput+0x255/0x2a0 [ 78.457372] __x64_sys_close+0x3d/0x80 [ 78.457544] do_syscall_64+0xa4/0x290 [ 78.457697] entry_SYSCALL_64_after_hwframe+0x77/0x7f [ 78.457883] RIP: 0033:0x7f2f227d1cc7 [ 78.458097] Code: 48 89 fa 4c 89 df e8 08 ae 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44f [ 78.458692] RSP: 002b:00007fffab36fb50 EFLAGS: 00000202 ORIG_RAX: 0000000000000003 [ 78.459155] RAX: ffffffffffffffda RBX: 00007f2f22741740 RCX: 00007f2f227d1cc7 [ 78.459400] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 [ 78.459648] RBP: 00007fffab36fba8 R08: 0000000000000000 R09: 0000000000000000 [ 78.459899] R10: 0000000000000000 R11: 0000000000000202 R12: 0000558298427128 [ 78.460212] R13: 00007f2f227416d0 R14: 00005582c72cf320 R15: 00005582c72cf320 [ 78.460470] </TASK> [ 78.460571] ---[ end trace 0000000000000000 ]--- The cdev is contained in the "gb_raw" structure, which is freed in the disconnect operation. When the cdev is released at a later time, cdev_put gets an address that points to freed memory. To fix this use-after-free, convert the struct device from a pointer to being embedded, that makes the lifetime of the cdev and of this device the same. Then, use cdev_device_add, which guarantees that the device won't be released until all references to the cdev are not released. Finally, delegate the freeing of the structure to the device release function, instead of freeing immediately in the disconnect callback. Fixes: e806c7fb8e9b ("greybus: raw: add raw greybus kernel driver") Signed-off-by: Damien Riégel <damien.riegel(a)silabs.com> --- drivers/staging/greybus/raw.c | 49 +++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 71de6776739..b92214f97e3 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -21,9 +21,8 @@ struct gb_raw { struct list_head list; int list_data; struct mutex list_lock; - dev_t dev; struct cdev cdev; - struct device *device; + struct device dev; }; struct raw_data { @@ -148,6 +147,13 @@ static int gb_raw_send(struct gb_raw *raw, u32 len, const char __user *data) return retval; } +static void raw_dev_release(struct device *dev) +{ + struct gb_raw *raw = dev_get_drvdata(dev); + + kfree(raw); +} + static int gb_raw_probe(struct gb_bundle *bundle, const struct greybus_bundle_id *id) { @@ -168,11 +174,14 @@ static int gb_raw_probe(struct gb_bundle *bundle, if (!raw) return -ENOMEM; + device_initialize(&raw->dev); + dev_set_drvdata(&raw->dev, raw); + connection = gb_connection_create(bundle, le16_to_cpu(cport_desc->id), gb_raw_request_handler); if (IS_ERR(connection)) { retval = PTR_ERR(connection); - goto error_free; + goto error_put_device; } INIT_LIST_HEAD(&raw->list); @@ -187,29 +196,26 @@ static int gb_raw_probe(struct gb_bundle *bundle, goto error_connection_destroy; } - raw->dev = MKDEV(raw_major, minor); + raw->dev.devt = MKDEV(raw_major, minor); + raw->dev.class = &raw_class; + raw->dev.parent = &connection->bundle->dev; + raw->dev.release = raw_dev_release; + retval = dev_set_name(&raw->dev, "gb!raw%d", minor); + if (retval) + goto error_remove_ida; + cdev_init(&raw->cdev, &raw_fops); retval = gb_connection_enable(connection); if (retval) goto error_remove_ida; - retval = cdev_add(&raw->cdev, raw->dev, 1); + retval = cdev_device_add(&raw->cdev, &raw->dev); if (retval) goto error_connection_disable; - raw->device = device_create(&raw_class, &connection->bundle->dev, - raw->dev, raw, "gb!raw%d", minor); - if (IS_ERR(raw->device)) { - retval = PTR_ERR(raw->device); - goto error_del_cdev; - } - return 0; -error_del_cdev: - cdev_del(&raw->cdev); - error_connection_disable: gb_connection_disable(connection); @@ -219,8 +225,8 @@ static int gb_raw_probe(struct gb_bundle *bundle, error_connection_destroy: gb_connection_destroy(connection); -error_free: - kfree(raw); +error_put_device: + put_device(&raw->dev); return retval; } @@ -231,11 +237,9 @@ static void gb_raw_disconnect(struct gb_bundle *bundle) struct raw_data *raw_data; struct raw_data *temp; - // FIXME - handle removing a connection when the char device node is open. - device_destroy(&raw_class, raw->dev); - cdev_del(&raw->cdev); + cdev_device_del(&raw->cdev, &raw->dev); gb_connection_disable(connection); - ida_free(&minors, MINOR(raw->dev)); + ida_free(&minors, MINOR(raw->dev.devt)); gb_connection_destroy(connection); mutex_lock(&raw->list_lock); @@ -244,8 +248,7 @@ static void gb_raw_disconnect(struct gb_bundle *bundle) kfree(raw_data); } mutex_unlock(&raw->list_lock); - - kfree(raw); + put_device(&raw->dev); } /* -- 2.52.0

6 months

[PATCH 1/2] staging: greybus: uart: fix style issues

by Shubham Chakraborty

Fix checkpatch.pl warnings by adding comments to mutex and spinlocks, and fixing alignment to match open parenthesis. Signed-off-by: Shubham Chakraborty <chakrabortyshubham66(a)gmail.com> --- drivers/staging/greybus/uart.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..1d2c4ef70865 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -50,12 +50,12 @@ struct gb_tty { unsigned int minor; unsigned char clocal; bool disconnected; - spinlock_t read_lock; - spinlock_t write_lock; + spinlock_t read_lock; /* protects read operations */ + spinlock_t write_lock; /* protects write operations */ struct async_icount iocount; struct async_icount oldcount; wait_queue_head_t wioctl; - struct mutex mutex; + struct mutex mutex; /* serializes port operations */ u8 ctrlin; /* input control lines */ u8 ctrlout; /* output control lines */ struct gb_uart_set_line_coding_request line_coding; @@ -318,7 +318,7 @@ static int gb_uart_wait_for_all_credits(struct gb_tty *gb_tty) return 0; ret = wait_for_completion_timeout(&gb_tty->credits_complete, - msecs_to_jiffies(GB_UART_CREDIT_WAIT_TIMEOUT_MSEC)); + msecs_to_jiffies(GB_UART_CREDIT_WAIT_TIMEOUT_MSEC)); if (!ret) { dev_err(&gb_tty->gbphy_dev->dev, "time out waiting for credits\n"); -- 2.53.0

6 months, 1 week

[PATCH] staging: greybus: audio_manager_module: make envp array const

by kunal km

From 630a0c3367818fa30032b85df71fbb03eaefb1b4 Mon Sep 17 00:00:00 2001 From: Kunal <kunalkmr9717(a)outlook.com> Date: Thu, 26 Feb 2026 11:31:47 +0000 Subject: [PATCH] staging: greybus: audio_manager_module: make envp array const Signed-off-by: Kunal <kunalkmr9717(a)outlook.com> --- drivers/staging/greybus/audio_manager_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c index e87b82ca6d8a..7a25af3421d8 100644 --- a/drivers/staging/greybus/audio_manager_module.c +++ b/drivers/staging/greybus/audio_manager_module.c @@ -159,7 +159,7 @@ static void send_add_uevent(struct gb_audio_manager_module *module) char ip_devices_string[64]; char op_devices_string[64]; - char *envp[] = { + char * const envp[] = { name_string, vid_string, pid_string, -- 2.43.0

6 months, 1 week

[PATCH] staging: greybus: audio_manager_module: make envp array const

by kunal km

6 months, 1 week

[PATCH] staging: greybus: arche: use sysfs_emit() instead of sprintf()

by Ruslan Valiyev

Replace sprintf() with sysfs_emit() in state_show() sysfs attribute callbacks in arche-platform.c and arche-apb-ctrl.c. sysfs_emit() is preferred over sprintf() in sysfs show functions because it is aware of the PAGE_SIZE buffer limit, preventing potential buffer overflows. This addresses checkpatch warnings about the use of sprintf() in sysfs show callbacks. Signed-off-by: Ruslan Valiyev <linuxoid(a)gmail.com> --- drivers/staging/greybus/arche-apb-ctrl.c | 10 +++++----- drivers/staging/greybus/arche-platform.c | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c index 33f26a65f..19a6e59b6 100644 --- a/drivers/staging/greybus/arche-apb-ctrl.c +++ b/drivers/staging/greybus/arche-apb-ctrl.c @@ -300,16 +300,16 @@ static ssize_t state_show(struct device *dev, switch (apb->state) { case ARCHE_PLATFORM_STATE_OFF: - return sprintf(buf, "off%s\n", + return sysfs_emit(buf, "off%s\n", apb->init_disabled ? ",disabled" : ""); case ARCHE_PLATFORM_STATE_ACTIVE: - return sprintf(buf, "active\n"); + return sysfs_emit(buf, "active\n"); case ARCHE_PLATFORM_STATE_STANDBY: - return sprintf(buf, "standby\n"); + return sysfs_emit(buf, "standby\n"); case ARCHE_PLATFORM_STATE_FW_FLASHING: - return sprintf(buf, "fw_flashing\n"); + return sysfs_emit(buf, "fw_flashing\n"); default: - return sprintf(buf, "unknown state\n"); + return sysfs_emit(buf, "unknown state\n"); } } diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c index f669a7e2e..de5de59ea 100644 --- a/drivers/staging/greybus/arche-platform.c +++ b/drivers/staging/greybus/arche-platform.c @@ -374,15 +374,15 @@ static ssize_t state_show(struct device *dev, switch (arche_pdata->state) { case ARCHE_PLATFORM_STATE_OFF: - return sprintf(buf, "off\n"); + return sysfs_emit(buf, "off\n"); case ARCHE_PLATFORM_STATE_ACTIVE: - return sprintf(buf, "active\n"); + return sysfs_emit(buf, "active\n"); case ARCHE_PLATFORM_STATE_STANDBY: - return sprintf(buf, "standby\n"); + return sysfs_emit(buf, "standby\n"); case ARCHE_PLATFORM_STATE_FW_FLASHING: - return sprintf(buf, "fw_flashing\n"); + return sysfs_emit(buf, "fw_flashing\n"); default: - return sprintf(buf, "unknown state\n"); + return sysfs_emit(buf, "unknown state\n"); } } -- 2.43.0

6 months, 1 week

[PATCH] staging: greybus: arche-platform: Use sysfs_emit instead of sprintf

by Shubham Chakraborty

Refactor sprintf to sysfs_emit in the show function of the arche platform driver. This follows the standard kernel practice of using sysfs_emit for sysfs attributes, ensuring consistent output formatting and newline handling. Signed-off-by: Shubham Chakraborty <chakrabortyshubham66(a)gmail.com> --- drivers/staging/greybus/arche-platform.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c index f669a7e2eb11..de5de59ea8ab 100644 --- a/drivers/staging/greybus/arche-platform.c +++ b/drivers/staging/greybus/arche-platform.c @@ -374,15 +374,15 @@ static ssize_t state_show(struct device *dev, switch (arche_pdata->state) { case ARCHE_PLATFORM_STATE_OFF: - return sprintf(buf, "off\n"); + return sysfs_emit(buf, "off\n"); case ARCHE_PLATFORM_STATE_ACTIVE: - return sprintf(buf, "active\n"); + return sysfs_emit(buf, "active\n"); case ARCHE_PLATFORM_STATE_STANDBY: - return sprintf(buf, "standby\n"); + return sysfs_emit(buf, "standby\n"); case ARCHE_PLATFORM_STATE_FW_FLASHING: - return sprintf(buf, "fw_flashing\n"); + return sysfs_emit(buf, "fw_flashing\n"); default: - return sprintf(buf, "unknown state\n"); + return sysfs_emit(buf, "unknown state\n"); } } -- 2.53.0

6 months, 1 week

[PATCH v3 0/2] staging: greybus: audio: remove dead code and clean up

by Hardik Phalet

This series cleans up dead code in the Greybus audio manager. gb_audio_manager_get_module() has no in-tree callers. The previously reported NULL dereference is therefore unreachable. Per review feedback, the unused function is removed to avoid carrying dead code. Patch 2 performs a small cleanup in the same area. Changes in v3: - Replaced the NULL-deref fix with removal of gb_audio_manager_get_module() since there are no in-tree callers (per Greg KH). - No functional changes otherwise. Thanks for the review. Signed-off-by: Hardik Phalet <hardik.phalet(a)pm.me> Hardik Phalet (2): staging: greybus: audio: remove unused gb_audio_manager_get_module() staging: greybus: audio: drop stale TODO comment drivers/staging/greybus/audio_manager.c | 12 ------------ drivers/staging/greybus/audio_manager.h | 7 ------- drivers/staging/greybus/audio_manager_module.c | 1 - 3 files changed, 20 deletions(-) -- 2.53.0

6 months, 1 week

[PATCH] staging: greybus: Use guard(mutex) in receive_data()

by Matvey Oborotov

Replace manual mutex_lock/unlock with guard(mutex) in raw.c receive_data(). This automates lock release on function exit, ensures that lock is released on early returns, and makes the code cleaner. Signed-off-by: Matvey Oborotov <oborotovmatvey(a)gmail.com> --- drivers/staging/greybus/raw.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 3027a2c25bcd..6c31bae0d8ea 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -59,34 +59,28 @@ static int receive_data(struct gb_raw *raw, u32 len, u8 *data) { struct raw_data *raw_data; struct device *dev = &raw->connection->bundle->dev; - int retval = 0; if (len > MAX_PACKET_SIZE) { dev_err(dev, "Too big of a data packet, rejected\n"); return -EINVAL; } - mutex_lock(&raw->list_lock); + guard(mutex)(&raw->list_lock); if ((raw->list_data + len) > MAX_DATA_SIZE) { dev_err(dev, "Too much data in receive buffer, now dropping packets\n"); - retval = -EINVAL; - goto exit; + return -EINVAL; } raw_data = kmalloc_flex(*raw_data, data, len); - if (!raw_data) { - retval = -ENOMEM; - goto exit; - } + if (!raw_data) + return -ENOMEM; raw->list_data += len; raw_data->len = len; memcpy(&raw_data->data[0], data, len); list_add_tail(&raw_data->entry, &raw->list); -exit: - mutex_unlock(&raw->list_lock); - return retval; + return 0; } static int gb_raw_request_handler(struct gb_operation *op) -- 2.43.0

6 months, 1 week

← Newer
1
...
12
13
14
15
16
17
18
...
86
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev