greybus-dev

greybus-dev@lists.linaro.org

12 participants
815 discussions

[PATCH v3] staging: greybus: uart: return tty_alloc_driver() errors

by Alfie Varghese

gb_tty_init() maps any tty_alloc_driver() failure to -ENOMEM. tty_alloc_driver() currently always returns -ENOMEM on failure, so this does not change behavior in practice. However, returning PTR_ERR(gb_tty_driver) is more correct and consistent with kernel conventions, preserving any future error codes the function might return. Signed-off-by: Alfie Varghese <alfievarghese22(a)gmail.com> --- v3: resend, v2 was corrupted in transit. No code changes. v2: updated commit message per Dan Carpenter's review to clarify that tty_alloc_driver() currently only returns -ENOMEM, making this a style fix rather than a behavioral change. --- drivers/staging/greybus/uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..24b4dab069c3 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -951,7 +951,7 @@ static int gb_tty_init(void) TTY_DRIVER_DYNAMIC_DEV); if (IS_ERR(gb_tty_driver)) { pr_err("Can not allocate tty driver\n"); - retval = -ENOMEM; + retval = PTR_ERR(gb_tty_driver); goto fail_unregister_dev; } -- 2.54.0.windows.1

1 day, 17 hours

[PATCH v2] staging: greybus: uart: return tty_alloc_driver() errors

by Alfie Varghese

gb_tty_init() maps any tty_alloc_driver() failure to -ENOMEM. tty_alloc_driver() currently always returns -ENOMEM on failure, so this does not change behavior in practice. However, returning PTR_ERR(gb_tty_driver) is more correct and consistent with kernel conventions, preserving any future error codes the function might return. Signed-off-by: Alfie Varghese <alfievarghese22(a)gmail.com> --- v2: updated commit message per Dan Carpenter's review to clarify that tty_alloc_driver() currently only returns -ENOMEM, making this a style fix rather than a behavioral change. drivers/staging/greybus/uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..24b4dab069c3 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -951,7 +951,7 @@ static int gb_tty_init(void) if (IS_ERR(gb_tty_driver)) { pr_err("Can not allocate tty driver\n"); - retval = -ENOMEM; + retval = PTR_ERR(gb_tty_driver); goto fail_unregister_dev; }

1 day, 23 hours

[PATCH] staging: greybus: uart: return tty_alloc_driver() errors

by Alfie Varghese

gb_tty_init() maps any tty_alloc_driver() failure to -ENOMEM. This loses the real errno returned by the driver core and makes failures harder to diagnose correctly. Return PTR_ERR(gb_tty_driver) instead so callers receive the actual failure reason while preserving the existing cleanup path. Signed-off-by: Alfie Varghese <alfievarghese22(a)gmail.com> --- drivers/staging/greybus/uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..24b4dab069c3 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -951,7 +951,7 @@ static int gb_tty_init(void) TTY_DRIVER_DYNAMIC_DEV); if (IS_ERR(gb_tty_driver)) { pr_err("Can not allocate tty driver\n"); - retval = -ENOMEM; + retval = PTR_ERR(gb_tty_driver); goto fail_unregister_dev; } -- 2.54.0.windows.1

1 day, 23 hours

[PATCH] staging: greybus: hid: fix SET_REPORT return value

by Hao-Qun Huang

__gb_hid_output_raw_report() stores the result of gb_hid_set_report() in ret and even adjusts it to account for the report ID byte, but then always returns 0. This hides Greybus transport errors from HID_REQ_SET_REPORT callers, and makes hidraw report zero bytes written to user space on success, although hid_hw_raw_request() is expected to return the number of bytes transferred or a negative errno. The sibling GET_REPORT path, __gb_hid_get_raw_report(), already follows this convention. Return ret like the other HID transport drivers do. Fixes: 96eab779e198 ("greybus: hid: add HID class driver") Cc: stable(a)vger.kernel.org Assisted-by: Claude:claude-fable-5 Signed-off-by: Hao-Qun Huang <alvinhuang0603(a)gmail.com> --- diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c index f1f9f6fbc00e..1d7186eecd23 100644 --- a/drivers/staging/greybus/hid.c +++ b/drivers/staging/greybus/hid.c @@ -256,7 +256,7 @@ static int __gb_hid_output_raw_report(struct hid_device *hid, __u8 *buf, if (report_id && ret >= 0) ret++; /* add report_id to the number of transferred bytes */ - return 0; + return ret; } static int gb_hid_raw_request(struct hid_device *hid, unsigned char reportnum,

5 days

[PATCH] staging: greybus: audio: bound topology parsing to the received buffer

by Muhammad Bilal

The Greybus audio topology parser trusts length and count fields taken straight from the module's topology blob and never checks them against the size of the buffer it actually allocated, leading to out-of-bounds reads of the kernel heap. gb_audio_gb_get_topology() reads a u16 size from the module, allocates a buffer of that size, fetches the topology into it, and then discards the size. gbaudio_tplg_parse_data() then walks that buffer using fields stored inside it: - gbaudio_tplg_process_header() computes the control, widget and route block offsets by adding the wire-supplied __le32 size_dais, size_controls and size_widgets onto the buffer base with no bound, so a module that reports a small allocation size but large block sizes moves those offsets far past the end of the buffer before they are dereferenced. - gbaudio_tplg_process_kcontrols(), _process_widgets() and _process_routes() iterate num_controls / num_widgets / num_routes (also from the blob) and advance a pointer by a per-element size that includes the __le16 names_length of an enumerated control, again with no check that the element stays inside the buffer. - gb_generate_enum_strings() loops over an attacker-controlled __le32 items count and, for each, scans for a NUL terminator with no end pointer, walking off the end of the buffer. A malicious or malfunctioning module can therefore make the parser read past the allocation. The wild block offsets are most likely to hit an unmapped page and oops (denial of service); the byte-at-a-time enum scan walks from a still-valid pointer and can copy adjacent heap bytes into ALSA control name strings, which are readable by unprivileged local users, so an information leak cannot be ruled out. Thread the allocated topology size from gb_audio_gb_get_topology() through to gbaudio_tplg_parse_data() and bound every walk against the end of the buffer: verify the block offsets are ordered and within the buffer (the "< previous" tests also catch a 32-bit unsigned wrap of the running offset), check each control, widget and route lies fully inside its block before use, and give gb_generate_enum_strings() an explicit end pointer plus an items-versus-names_length sanity check. Fixes: 6339d2322c47 ("greybus: audio: Add topology parser for GB codec") Cc: stable(a)vger.kernel.org Signed-off-by: Muhammad Bilal <meatuni001(a)gmail.com> --- drivers/staging/greybus/audio_codec.h | 4 +- drivers/staging/greybus/audio_gb.c | 13 ++- drivers/staging/greybus/audio_module.c | 6 +- drivers/staging/greybus/audio_topology.c | 135 ++++++++++++++++++----- 4 files changed, 120 insertions(+), 38 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h index f3f7a7ec6be40..f9225cf52d843 100644 --- a/drivers/staging/greybus/audio_codec.h +++ b/drivers/staging/greybus/audio_codec.h @@ -167,7 +167,7 @@ struct gbaudio_module_info { }; int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, - struct gb_audio_topology *tplg_data); + struct gb_audio_topology *tplg_data, size_t size); void gbaudio_tplg_release(struct gbaudio_module_info *module); int gbaudio_module_update(struct gbaudio_codec_info *codec, @@ -179,7 +179,7 @@ void gbaudio_unregister_module(struct gbaudio_module_info *module); /* protocol related */ int gb_audio_gb_get_topology(struct gb_connection *connection, - struct gb_audio_topology **topology); + struct gb_audio_topology **topology, size_t *size); int gb_audio_gb_get_control(struct gb_connection *connection, u8 control_id, u8 index, struct gb_audio_ctl_elem_value *value); diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 9d8994fdb41a2..0811652bc417f 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -10,11 +10,11 @@ /* TODO: Split into separate calls */ int gb_audio_gb_get_topology(struct gb_connection *connection, - struct gb_audio_topology **topology) + struct gb_audio_topology **topology, size_t *size) { struct gb_audio_get_topology_size_response size_resp; struct gb_audio_topology *topo; - u16 size; + u16 tplg_size; int ret; ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, @@ -22,22 +22,23 @@ int gb_audio_gb_get_topology(struct gb_connection *connection, if (ret) return ret; - size = le16_to_cpu(size_resp.size); - if (size < sizeof(*topo)) + tplg_size = le16_to_cpu(size_resp.size); + if (tplg_size < sizeof(*topo)) return -ENODATA; - topo = kzalloc(size, GFP_KERNEL); + topo = kzalloc(tplg_size, GFP_KERNEL); if (!topo) return -ENOMEM; ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0, - topo, size); + topo, tplg_size); if (ret) { kfree(topo); return ret; } *topology = topo; + *size = tplg_size; return 0; } diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c index 12c376c477b3c..9367ab6debdbe 100644 --- a/drivers/staging/greybus/audio_module.c +++ b/drivers/staging/greybus/audio_module.c @@ -240,6 +240,7 @@ static int gb_audio_probe(struct gb_bundle *bundle, struct gbaudio_data_connection *dai, *_dai; int ret, i; struct gb_audio_topology *topology; + size_t tplg_size; /* There should be at least one Management and one Data cport */ if (bundle->num_cports < 2) @@ -308,14 +309,15 @@ static int gb_audio_probe(struct gb_bundle *bundle, * FIXME: malloc for topology happens via audio_gb driver * should be done within codec driver itself */ - ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, &topology); + ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, &topology, + &tplg_size); if (ret) { dev_err(dev, "%d:Error while fetching topology\n", ret); goto disable_connection; } /* process topology data */ - ret = gbaudio_tplg_parse_data(gbmodule, topology); + ret = gbaudio_tplg_parse_data(gbmodule, topology, tplg_size); if (ret) { dev_err(dev, "%d:Error while parsing topology data\n", ret); diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c index 76146f91cddcc..4095e6c741efa 100644 --- a/drivers/staging/greybus/audio_topology.c +++ b/drivers/staging/greybus/audio_topology.c @@ -134,21 +134,35 @@ static const char **gb_generate_enum_strings(struct gbaudio_module_info *gb, struct gb_audio_enumerated *gbenum) { const char **strings; - int i; unsigned int items; - __u8 *data; + u16 names_length; + const __u8 *data; + const __u8 *end; + int i; items = le32_to_cpu(gbenum->items); + names_length = le16_to_cpu(gbenum->names_length); + data = gbenum->names; + end = data + names_length; + + /* + * Each enumerated value is a NUL-terminated string occupying at least + * one byte, so a valid names block cannot hold more items than it has + * bytes. This also bounds the devm_kcalloc() request below. + */ + if (items > names_length) + return NULL; + strings = devm_kcalloc(gb->dev, items, sizeof(char *), GFP_KERNEL); if (!strings) return NULL; - data = gbenum->names; - for (i = 0; i < items; i++) { strings[i] = (const char *)data; - while (*data != '\0') + while (data < end && *data != '\0') data++; + if (data == end) + return NULL; data++; } @@ -1009,9 +1023,40 @@ static const struct snd_soc_dapm_widget gbaudio_widgets[] = { SND_SOC_DAPM_POST_PMD), }; +/* + * Return the on-wire size of the topology control at @curr, in bytes, after + * verifying that the whole control - including its variable-length enum names + * block - lies within [@curr, @end). Returns a negative errno on overrun. + */ +static int gbaudio_control_size(struct gb_audio_control *curr, const u8 *end) +{ + size_t csize; + + /* + * Enough of the control must be present to read its id and name and + * the fixed part of the enumerated descriptor (items, names_length). + */ + csize = offsetof(struct gb_audio_control, info); + csize += offsetof(struct gb_audio_ctl_elem_info, value); + csize += offsetof(struct gb_audio_enumerated, names); + if ((u8 *)curr + csize > end) + return -EINVAL; + + if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) + csize += le16_to_cpu(curr->info.value.enumerated.names_length); + else + csize = sizeof(struct gb_audio_control); + + if ((u8 *)curr + csize > end) + return -EINVAL; + + return csize; +} + static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, struct snd_soc_dapm_widget *dw, - struct gb_audio_widget *w, int *w_size) + struct gb_audio_widget *w, int *w_size, + const u8 *end) { int i, ret, csize; struct snd_kcontrol_new *widget_kctls; @@ -1040,6 +1085,11 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, /* create relevant kcontrols */ curr = w->ctl; for (i = 0; i < w->ncontrols; i++) { + ret = gbaudio_control_size(curr, end); + if (ret < 0) + goto error; + csize = ret; + ret = gbaudio_tplg_create_wcontrol(module, &widget_kctls[i], curr); if (ret) { @@ -1063,10 +1113,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, struct gb_audio_enumerated *gbenum = &curr->info.value.enumerated; - csize = offsetof(struct gb_audio_control, info); - csize += offsetof(struct gb_audio_ctl_elem_info, value); - csize += offsetof(struct gb_audio_enumerated, names); - csize += le16_to_cpu(gbenum->names_length); control->texts = (const char * const *) gb_generate_enum_strings(module, gbenum); if (!control->texts) { @@ -1074,8 +1120,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, goto error; } control->items = le32_to_cpu(gbenum->items); - } else { - csize = sizeof(struct gb_audio_control); } *w_size += csize; @@ -1136,7 +1180,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, } static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, - struct gb_audio_control *controls) + struct gb_audio_control *controls, + const u8 *end) { int i, csize, ret; struct snd_kcontrol_new *dapm_kctls; @@ -1152,6 +1197,11 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, curr = controls; for (i = 0; i < module->num_controls; i++) { + ret = gbaudio_control_size(curr, end); + if (ret < 0) + goto error; + csize = ret; + ret = gbaudio_tplg_create_kcontrol(module, &dapm_kctls[i], curr); if (ret) { @@ -1176,10 +1226,6 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, struct gb_audio_enumerated *gbenum = &curr->info.value.enumerated; - csize = offsetof(struct gb_audio_control, info); - csize += offsetof(struct gb_audio_ctl_elem_info, value); - csize += offsetof(struct gb_audio_enumerated, names); - csize += le16_to_cpu(gbenum->names_length); control->texts = (const char * const *) gb_generate_enum_strings(module, gbenum); if (!control->texts) { @@ -1187,8 +1233,6 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, goto error; } control->items = le32_to_cpu(gbenum->items); - } else { - csize = sizeof(struct gb_audio_control); } list_add(&control->list, &module->ctl_list); @@ -1210,7 +1254,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, } static int gbaudio_tplg_process_widgets(struct gbaudio_module_info *module, - struct gb_audio_widget *widgets) + struct gb_audio_widget *widgets, + const u8 *end) { int i, ret, w_size; struct snd_soc_dapm_widget *dapm_widgets; @@ -1225,8 +1270,13 @@ static int gbaudio_tplg_process_widgets(struct gbaudio_module_info *module, curr = widgets; for (i = 0; i < module->num_dapm_widgets; i++) { + /* The fixed part of the widget must lie within the buffer. */ + if ((u8 *)curr + sizeof(struct gb_audio_widget) > end) { + ret = -EINVAL; + goto error; + } ret = gbaudio_tplg_create_widget(module, &dapm_widgets[i], - curr, &w_size); + curr, &w_size, end); if (ret) { dev_err(module->dev, "%s:%d type not supported\n", curr->name, curr->type); @@ -1259,7 +1309,8 @@ static int gbaudio_tplg_process_widgets(struct gbaudio_module_info *module, } static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, - struct gb_audio_route *routes) + struct gb_audio_route *routes, + const u8 *end) { int i, ret; struct snd_soc_dapm_route *dapm_routes; @@ -1275,6 +1326,10 @@ static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, curr = routes; for (i = 0; i < module->num_dapm_routes; i++) { + if ((u8 *)curr + sizeof(struct gb_audio_route) > end) { + ret = -EINVAL; + goto error; + } dapm_routes->sink = gbaudio_map_widgetid(module, curr->destination_id); if (!dapm_routes->sink) { @@ -1320,8 +1375,12 @@ static int gbaudio_tplg_process_routes(struct gbaudio_module_info *module, } static int gbaudio_tplg_process_header(struct gbaudio_module_info *module, - struct gb_audio_topology *tplg_data) + struct gb_audio_topology *tplg_data, + size_t size) { + unsigned long tplg_start = (unsigned long)tplg_data; + unsigned long tplg_end = tplg_start + size; + /* fetch no. of kcontrols, widgets & routes */ module->num_controls = tplg_data->num_controls; module->num_dapm_widgets = tplg_data->num_widgets; @@ -1336,6 +1395,20 @@ static int gbaudio_tplg_process_header(struct gbaudio_module_info *module, module->route_offset = module->widget_offset + le32_to_cpu(tplg_data->size_widgets); + /* + * The DAI, control, widget and route blocks are concatenated in that + * order after the header. Their sizes come straight off the wire and + * are attacker-controlled, so verify the resulting block boundaries + * are ordered and stay within the allocated topology buffer. The + * "< previous" tests also reject an unsigned wrap of the running + * offset on 32-bit builds. + */ + if (module->control_offset < module->dai_offset || + module->widget_offset < module->control_offset || + module->route_offset < module->widget_offset || + module->route_offset > tplg_end) + return -EINVAL; + dev_dbg(module->dev, "DAI offset is 0x%lx\n", module->dai_offset); dev_dbg(module->dev, "control offset is %lx\n", module->control_offset); @@ -1346,7 +1419,7 @@ static int gbaudio_tplg_process_header(struct gbaudio_module_info *module, } int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, - struct gb_audio_topology *tplg_data) + struct gb_audio_topology *tplg_data, size_t size) { int ret; struct gb_audio_control *controls; @@ -1357,7 +1430,10 @@ int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, if (!tplg_data) return -EINVAL; - ret = gbaudio_tplg_process_header(module, tplg_data); + if (size < sizeof(*tplg_data)) + return -EINVAL; + + ret = gbaudio_tplg_process_header(module, tplg_data, size); if (ret) { dev_err(module->dev, "%d: Error in parsing topology header\n", ret); @@ -1366,7 +1442,8 @@ int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, /* process control */ controls = (struct gb_audio_control *)module->control_offset; - ret = gbaudio_tplg_process_kcontrols(module, controls); + ret = gbaudio_tplg_process_kcontrols(module, controls, + (const u8 *)module->widget_offset); if (ret) { dev_err(module->dev, "%d: Error in parsing controls data\n", ret); @@ -1376,7 +1453,8 @@ int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, /* process widgets */ widgets = (struct gb_audio_widget *)module->widget_offset; - ret = gbaudio_tplg_process_widgets(module, widgets); + ret = gbaudio_tplg_process_widgets(module, widgets, + (const u8 *)module->route_offset); if (ret) { dev_err(module->dev, "%d: Error in parsing widgets data\n", ret); @@ -1386,7 +1464,8 @@ int gbaudio_tplg_parse_data(struct gbaudio_module_info *module, /* process route */ routes = (struct gb_audio_route *)module->route_offset; - ret = gbaudio_tplg_process_routes(module, routes); + ret = gbaudio_tplg_process_routes(module, routes, + (const u8 *)tplg_data + size); if (ret) { dev_err(module->dev, "%d: Error in parsing routes data\n", ret); -- 2.55.0

5 days, 10 hours

[PATCH 0/2] Bring includes in linux/kmod.h up to date

by Petr Pavlu

The usermode helper declarations were previously provided by linux/kmod.h but commit c1f3fa2a4fde ("kmod: split off umh headers into its own file") moved them to linux/umh.h in 2017. Add explicit includes of linux/umh.h to files that use usermode helpers and remove linux/kmod.h where it is no longer needed. Then clean up linux/kmod.h so that it includes only the headers that it actually requires, importantly removing the compat linux/umh.h include. Apologies for the wide distribution. This cleanup is motivated by trying to reduce the preprocessed size of linux/module.h, which includes linux/kmod.h. The linux/module.h header is included by every *.mod.c file to provide `struct module` and other related definitions, so it should avoid pulling in unnecessary dependencies. Note that this series doesn't immediately improve the situation, since most of the files included by linux/kmod.h are, for now, also included by linux/module.h through other paths. Petr Pavlu (2): umh, treewide: Explicitly include linux/umh.h where needed module: Bring includes in linux/kmod.h up to date arch/x86/kernel/cpu/mce/dev-mcelog.c | 2 +- drivers/block/drbd/drbd_nl.c | 1 + drivers/greybus/svc_watchdog.c | 1 + drivers/macintosh/windfarm_core.c | 1 + drivers/pnp/pnpbios/core.c | 2 +- drivers/video/fbdev/uvesafb.c | 1 + fs/coredump.c | 2 +- fs/nfs/cache_lib.c | 2 +- fs/nfsd/nfs4layouts.c | 2 +- fs/nfsd/nfs4recover.c | 1 + fs/ocfs2/stackglue.c | 1 + include/linux/kmod.h | 12 ++---------- kernel/cgroup/cgroup-v1.c | 1 + kernel/module/kmod.c | 1 + kernel/power/process.c | 2 +- kernel/reboot.c | 2 +- kernel/umh.c | 2 +- lib/kobject_uevent.c | 2 +- net/bridge/br_stp_if.c | 2 +- security/keys/request_key.c | 2 +- security/tomoyo/common.h | 2 +- 21 files changed, 22 insertions(+), 22 deletions(-) base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482 -- 2.54.0

5 days, 18 hours

[PATCH] staging: greybus: audio: split topology get into size and data calls

by adi25charis＠gmail.com

From: Aditya Chari S <adi25charis(a)gmail.com> gb_audio_gb_get_topology() combined three separate responsibilities into a single call: querying the topology size, allocating a buffer for it, and fetching the topology data into that buffer. This left callers with no way to perform any of these steps independently, and forced the kzalloc() allocation to live inside the protocol-layer driver rather than the caller, as already flagged by a FIXME comment at the call site in audio_module.c. Split the function into two: gb_audio_gb_get_topology_size() - queries only the topology size gb_audio_gb_get_topology() - fetches topology data into a caller-supplied buffer of a given size Update the only caller, gb_audio_probe() in audio_module.c, to query the size first, allocate the topology buffer itself, then fetch the data into it, freeing the buffer via the existing free_topology error path on failure. This resolves both the "TODO: Split into separate calls" comment above the original function in audio_gb.c and the FIXME comment at the call site in audio_module.c, both of which are removed as part of this change. No functional change in behavior for the existing probe path. Compile-tested with W=1, sparse (C=2), and checkpatch.pl; all clean on the three changed files (audio_gb.c, audio_module.c, audio_codec.h). Signed-off-by: Aditya Chari S <adi25charis(a)gmail.com> --- drivers/staging/greybus/audio_codec.h | 4 +++- drivers/staging/greybus/audio_gb.c | 33 ++++++++++---------------- drivers/staging/greybus/audio_module.c | 21 +++++++++++----- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h index f3f7a7ec6..be5a2a86b 100644 --- a/drivers/staging/greybus/audio_codec.h +++ b/drivers/staging/greybus/audio_codec.h @@ -178,8 +178,10 @@ int gbaudio_register_module(struct gbaudio_module_info *module); void gbaudio_unregister_module(struct gbaudio_module_info *module); /* protocol related */ +int gb_audio_gb_get_topology_size(struct gb_connection *connection, + u16 *size); int gb_audio_gb_get_topology(struct gb_connection *connection, - struct gb_audio_topology **topology); + struct gb_audio_topology *topology, u16 size); int gb_audio_gb_get_control(struct gb_connection *connection, u8 control_id, u8 index, struct gb_audio_ctl_elem_value *value); diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 9d8994fdb..e6356643d 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -8,13 +8,10 @@ #include <linux/greybus.h> #include "audio_codec.h" -/* TODO: Split into separate calls */ -int gb_audio_gb_get_topology(struct gb_connection *connection, - struct gb_audio_topology **topology) +int gb_audio_gb_get_topology_size(struct gb_connection *connection, + u16 *size) { struct gb_audio_get_topology_size_response size_resp; - struct gb_audio_topology *topo; - u16 size; int ret; ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE, @@ -22,24 +19,20 @@ int gb_audio_gb_get_topology(struct gb_connection *connection, if (ret) return ret; - size = le16_to_cpu(size_resp.size); - if (size < sizeof(*topo)) - return -ENODATA; - - topo = kzalloc(size, GFP_KERNEL); - if (!topo) - return -ENOMEM; + *size = le16_to_cpu(size_resp.size); - ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0, - topo, size); - if (ret) { - kfree(topo); - return ret; - } + return 0; +} +EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology_size); - *topology = topo; +int gb_audio_gb_get_topology(struct gb_connection *connection, + struct gb_audio_topology *topology, u16 size) +{ + if (size < sizeof(*topology)) + return -ENODATA; - return 0; + return gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0, + topology, size); } EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology); diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c index 12c376c47..1163cf093 100644 --- a/drivers/staging/greybus/audio_module.c +++ b/drivers/staging/greybus/audio_module.c @@ -239,6 +239,7 @@ static int gb_audio_probe(struct gb_bundle *bundle, struct gb_audio_manager_module_descriptor desc; struct gbaudio_data_connection *dai, *_dai; int ret, i; + u16 size; struct gb_audio_topology *topology; /* There should be at least one Management and one Data cport */ @@ -304,16 +305,24 @@ static int gb_audio_probe(struct gb_bundle *bundle, } gbmodule->dev_id = gbmodule->mgmt_connection->intf->interface_id; - /* - * FIXME: malloc for topology happens via audio_gb driver - * should be done within codec driver itself - */ - ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, &topology); + ret = gb_audio_gb_get_topology_size(gbmodule->mgmt_connection, &size); if (ret) { - dev_err(dev, "%d:Error while fetching topology\n", ret); + dev_err(dev, "%d:Error while fetching topology size\n", ret); + goto disable_connection; + } + + topology = kzalloc(size, GFP_KERNEL); + if (!topology) { + ret = -ENOMEM; goto disable_connection; } + ret = gb_audio_gb_get_topology(gbmodule->mgmt_connection, topology, size); + if (ret) { + dev_err(dev, "%d:Error while fetching topology\n", ret); + goto free_topology; + } + /* process topology data */ ret = gbaudio_tplg_parse_data(gbmodule, topology); if (ret) { -- 2.53.0

1 week, 1 day

[RFC] staging: greybus: arche still depends on missing USB3613 provider

by Pengpeng Hou

Hi, while auditing conditional provider/header contracts, I noticed that Greybus Arche still appears to describe a USB3613 provider world that is absent from current mainline. drivers/staging/greybus/Kconfig still has: depends on USB_HSIC_USB3613 || COMPILE_TEST and drivers/staging/greybus/arche-platform.c still conditionally includes the USB3613 header and calls usb3613_hub_mode_ctrl() when CONFIG_USB_HSIC_USB3613 is enabled. However, the current tree does not appear to provide include/linux/usb/usb3613.h or a Kconfig provider for USB_HSIC_USB3613. I am not sending a patch yet because this is staging/hardware policy sensitive. The possible directions seem to be: 1. restore or move the USB3613 provider/header if the hardware path is still intended; 2. remove the stale USB3613 integration path and rely on the local stub; 3. change the Kconfig dependency to describe only current supported worlds; or 4. keep the contract if an out-of-tree provider is intentionally expected. Could you advise which direction is expected for Arche? This is static source/Kconfig/header analysis only. I have not tested Arche hardware. Thanks, Pengpeng

1 week, 1 day

[staging] staging: greybus: loopback: add missing comment for mutex

by Arnav Kapoor

Add a comment for the mutex as suggested by checkpatch. Signed-off-by: Arnav Kapoor <kapoorarnav43(a)gmail.com> --- drivers/staging/greybus/loopback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index ea57b1f5d..bf827e672 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -61,7 +61,7 @@ struct gb_loopback { struct dentry *file; struct kfifo kfifo_lat; - struct mutex mutex; + struct mutex mutex; /* protects the entire structure */ struct task_struct *task; struct device *dev; wait_queue_head_t wq; -- 2.53.0

1 week, 1 day

[PATCH v1] greybus: Drop #include of <linux/mod_devicetable.h>

by Uwe Kleine-König (The Capable Hub)

The header itself also includes <linux/types.h> and additional to that doesn't make use of any symbol defined (transitively) by <linux/mod_devicetable.h>. Also the .c files that include that header don't need it (there is no direct include, only via <linux/greybus.h>): $ git grep -l greybus\\.h | xargs grep -E "\<(acpi_device_id|amba_id|ap_device_id|apr_device_id|auxiliary_device_id|bcma_device_id|ccw_device_id|cdx_device_id|coreboot_device_id|css_device_id|dfl_device_id|dmi_(device|system)_id|eisa_device_id|fsl_mc_device_id|hda_device_id|hid_device_id|hv_vmbus_device_id|i2c_device_id|i3c_device_id|ieee1394_device_id|input_device_id|ipack_device_id|isapnp_device_id|ishtp_device_id|mcb_device_id|mdio_device_id|mei_cl_device_id|mhi_device_id|mips_cdmm_device_id|of_device_id|parisc_device_id|pci_device_id|pci_epf_device_id|pcmcia_device_id|platform_device_id|pnp_(card_)?device_id|rio_device_id|rpmsg_device_id|sdio_device_id|sdw_device_id|serio_device_id|slim_device_id|spi_device_id|spmi_device_id|ssam_device_id|ssb_device_id|tb_service_id|tee_client_device_id|typec_device_id|ulpi_device_id|usb_device_id|vchiq_device_id|virtio_device_id|wmi_device_id|x86_(cpu|device)_id|zorro_device_id|cpu_feature)\>" drivers/greybus/es2.c:static const struct usb_device_id id_table[] = { drivers/greybus/es2.c: const struct usb_device_id *id) drivers/greybus/gb-beagleplay.c:static const struct of_device_id gb_beagleplay_of_match[] = { drivers/staging/greybus/arche-platform.c:static const struct of_device_id arche_platform_of_match[] = { drivers/greybus/es2.c includes <linux/usb.h>, drivers/greybus/gb-beagleplay.c includes <linux/serdev.h> which provides of_device_id via <linux/device.h>, similar drivers/staging/greybus/arche-platform.c includes <linux/platform_device.h> which also provides of_device_id. So the #include can go away without further adaption. Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig(a)baylibre.com> --- include/linux/greybus/greybus_id.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/linux/greybus/greybus_id.h b/include/linux/greybus/greybus_id.h index f4c8440093e4..72f330a35569 100644 --- a/include/linux/greybus/greybus_id.h +++ b/include/linux/greybus/greybus_id.h @@ -1,14 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* FIXME - * move this to include/linux/mod_devicetable.h when merging + * move this to include/linux/device-id/greybus.h when merging */ #ifndef __LINUX_GREYBUS_ID_H #define __LINUX_GREYBUS_ID_H #include <linux/types.h> -#include <linux/mod_devicetable.h> - struct greybus_bundle_id { __u16 match_flags; base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda -- 2.55.0.11.g153666a7d9bb

1 week, 2 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev