greybus-dev June 2022

greybus-dev@lists.linaro.org

9 participants
8 discussions

Unitialized Variable and Null Pointer Dereference bug in gb_bootrom_get_firmware

by Dongliang Mu

Hi maintainers, I would like to send one bug report. In gb_bootrom_get_firmware, if the first branch is satisfied, it will go to queue_work, leading to the dereference of uninitialized const variable "fw". If the second branch is satisfied, it will go to unlock with fw as NULL pointer, leading to a NULL Pointer Dereference. The Fixes commit should be [1], introducing the dereference of "fw" in the error handling code. I am not sure how to fix this bug. Any comment on removing the dereference of fw? [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?… -- My best regards to you. No System Is Safe! Dongliang Mu

2 years, 6 months

[PATCH] drivers: greybus: Directly use ida_alloc()/free()

by keliu

Use ida_alloc()/ida_free() instead of deprecated ida_simple_get()/ida_simple_remove() . Signed-off-by: keliu <liuke94(a)huawei.com> --- drivers/greybus/es2.c | 4 ++-- drivers/greybus/hd.c | 12 ++++++------ drivers/greybus/interface.c | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/greybus/es2.c b/drivers/greybus/es2.c index e89cca015095..c861fb2acd8a 100644 --- a/drivers/greybus/es2.c +++ b/drivers/greybus/es2.c @@ -522,7 +522,7 @@ static int es2_cport_allocate(struct gb_host_device *hd, int cport_id, return -EINVAL; } - return ida_simple_get(id_map, ida_start, ida_end, GFP_KERNEL); + return ida_alloc_range(id_map, ida_start, ida_end - 1, GFP_KERNEL); } static void es2_cport_release(struct gb_host_device *hd, u16 cport_id) @@ -535,7 +535,7 @@ static void es2_cport_release(struct gb_host_device *hd, u16 cport_id) return; } - ida_simple_remove(&hd->cport_id_map, cport_id); + ida_free(&hd->cport_id_map, cport_id); } static int cport_enable(struct gb_host_device *hd, u16 cport_id, diff --git a/drivers/greybus/hd.c b/drivers/greybus/hd.c index 72b21bf2d7d3..6ff5b0cfd539 100644 --- a/drivers/greybus/hd.c +++ b/drivers/greybus/hd.c @@ -50,7 +50,7 @@ int gb_hd_cport_reserve(struct gb_host_device *hd, u16 cport_id) struct ida *id_map = &hd->cport_id_map; int ret; - ret = ida_simple_get(id_map, cport_id, cport_id + 1, GFP_KERNEL); + ret = ida_alloc_range(id_map, cport_id, cport_id, GFP_KERNEL); if (ret < 0) { dev_err(&hd->dev, "failed to reserve cport %u\n", cport_id); return ret; @@ -64,7 +64,7 @@ void gb_hd_cport_release_reserved(struct gb_host_device *hd, u16 cport_id) { struct ida *id_map = &hd->cport_id_map; - ida_simple_remove(id_map, cport_id); + ida_free(id_map, cport_id); } EXPORT_SYMBOL_GPL(gb_hd_cport_release_reserved); @@ -89,7 +89,7 @@ int gb_hd_cport_allocate(struct gb_host_device *hd, int cport_id, return -EINVAL; } - return ida_simple_get(id_map, ida_start, ida_end, GFP_KERNEL); + return ida_alloc_range(id_map, ida_start, ida_end - 1, GFP_KERNEL); } /* Locking: Caller guarantees serialisation */ @@ -100,7 +100,7 @@ void gb_hd_cport_release(struct gb_host_device *hd, u16 cport_id) return; } - ida_simple_remove(&hd->cport_id_map, cport_id); + ida_free(&hd->cport_id_map, cport_id); } static void gb_hd_release(struct device *dev) @@ -111,7 +111,7 @@ static void gb_hd_release(struct device *dev) if (hd->svc) gb_svc_put(hd->svc); - ida_simple_remove(&gb_hd_bus_id_map, hd->bus_id); + ida_free(&gb_hd_bus_id_map, hd->bus_id); ida_destroy(&hd->cport_id_map); kfree(hd); } @@ -162,7 +162,7 @@ struct gb_host_device *gb_hd_create(struct gb_hd_driver *driver, if (!hd) return ERR_PTR(-ENOMEM); - ret = ida_simple_get(&gb_hd_bus_id_map, 1, 0, GFP_KERNEL); + ret = ida_alloc_min(&gb_hd_bus_id_map, 1, GFP_KERNEL); if (ret < 0) { kfree(hd); return ERR_PTR(ret); diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c index 9ec949a438ef..f685e5f7b7b1 100644 --- a/drivers/greybus/interface.c +++ b/drivers/greybus/interface.c @@ -131,8 +131,8 @@ static int gb_interface_route_create(struct gb_interface *intf) int ret; /* Allocate an interface device id. */ - ret = ida_simple_get(&svc->device_id_map, - GB_SVC_DEVICE_ID_MIN, GB_SVC_DEVICE_ID_MAX + 1, + ret = ida_alloc_range(&svc->device_id_map, + GB_SVC_DEVICE_ID_MIN, GB_SVC_DEVICE_ID_MAX, GFP_KERNEL); if (ret < 0) { dev_err(&intf->dev, "failed to allocate device id: %d\n", ret); @@ -165,7 +165,7 @@ static int gb_interface_route_create(struct gb_interface *intf) * XXX anymore. */ err_ida_remove: - ida_simple_remove(&svc->device_id_map, device_id); + ida_free(&svc->device_id_map, device_id); return ret; } @@ -178,7 +178,7 @@ static void gb_interface_route_destroy(struct gb_interface *intf) return; gb_svc_route_destroy(svc, svc->ap_intf_id, intf->interface_id); - ida_simple_remove(&svc->device_id_map, intf->device_id); + ida_free(&svc->device_id_map, intf->device_id); intf->device_id = GB_INTERFACE_DEVICE_ID_BAD; } -- 2.25.1

2 years, 6 months

[PATCH] staging: greybus: Directly use ida_alloc()/free()

by keliu

Use ida_alloc()/ida_free() instead of deprecated ida_simple_get()/ida_simple_remove() . Signed-off-by: keliu <liuke94(a)huawei.com> --- drivers/staging/greybus/audio_manager.c | 8 ++++---- drivers/staging/greybus/authentication.c | 4 ++-- drivers/staging/greybus/fw-download.c | 4 ++-- drivers/staging/greybus/fw-management.c | 12 ++++++------ drivers/staging/greybus/gbphy.c | 4 ++-- drivers/staging/greybus/loopback.c | 6 +++--- drivers/staging/greybus/raw.c | 6 +++--- drivers/staging/greybus/vibrator.c | 6 +++--- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/staging/greybus/audio_manager.c b/drivers/staging/greybus/audio_manager.c index 9a3f7c034ab4..fa43d35bbcec 100644 --- a/drivers/staging/greybus/audio_manager.c +++ b/drivers/staging/greybus/audio_manager.c @@ -44,14 +44,14 @@ int gb_audio_manager_add(struct gb_audio_manager_module_descriptor *desc) int id; int err; - id = ida_simple_get(&module_id, 0, 0, GFP_KERNEL); + id = ida_alloc(&module_id, GFP_KERNEL); if (id < 0) return id; err = gb_audio_manager_module_create(&module, manager_kset, id, desc); if (err) { - ida_simple_remove(&module_id, id); + ida_free(&module_id, id); return err; } @@ -78,7 +78,7 @@ int gb_audio_manager_remove(int id) list_del(&module->list); kobject_put(&module->kobj); up_write(&modules_rwsem); - ida_simple_remove(&module_id, id); + ida_free(&module_id, id); return 0; } EXPORT_SYMBOL_GPL(gb_audio_manager_remove); @@ -92,7 +92,7 @@ void gb_audio_manager_remove_all(void) list_for_each_entry_safe(module, next, &modules_list, list) { list_del(&module->list); - ida_simple_remove(&module_id, module->id); + ida_free(&module_id, module->id); kobject_put(&module->kobj); } diff --git a/drivers/staging/greybus/authentication.c b/drivers/staging/greybus/authentication.c index 297e69f011c7..01dd1cd958ea 100644 --- a/drivers/staging/greybus/authentication.c +++ b/drivers/staging/greybus/authentication.c @@ -348,7 +348,7 @@ int gb_cap_connection_init(struct gb_connection *connection) err_del_cdev: cdev_del(&cap->cdev); err_remove_ida: - ida_simple_remove(&cap_minors_map, minor); + ida_free(&cap_minors_map, minor); err_connection_disable: gb_connection_disable(connection); err_list_del: @@ -372,7 +372,7 @@ void gb_cap_connection_exit(struct gb_connection *connection) device_destroy(cap_class, cap->dev_num); cdev_del(&cap->cdev); - ida_simple_remove(&cap_minors_map, MINOR(cap->dev_num)); + ida_free(&cap_minors_map, MINOR(cap->dev_num)); /* * Disallow any new ioctl operations on the char device and wait for diff --git a/drivers/staging/greybus/fw-download.c b/drivers/staging/greybus/fw-download.c index 543692c567f9..83652cb04d53 100644 --- a/drivers/staging/greybus/fw-download.c +++ b/drivers/staging/greybus/fw-download.c @@ -63,7 +63,7 @@ static void fw_req_release(struct kref *kref) * just hope that it never happens. */ if (!fw_req->timedout) - ida_simple_remove(&fw_req->fw_download->id_map, + ida_free(&fw_req->fw_download->id_map, fw_req->firmware_id); kfree(fw_req); @@ -212,7 +212,7 @@ static struct fw_request *find_firmware(struct fw_download *fw_download, return fw_req; err_free_id: - ida_simple_remove(&fw_download->id_map, fw_req->firmware_id); + ida_free(&fw_download->id_map, fw_req->firmware_id); err_free_req: kfree(fw_req); diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c index 687c6405c65b..a386a5607c7e 100644 --- a/drivers/staging/greybus/fw-management.c +++ b/drivers/staging/greybus/fw-management.c @@ -177,7 +177,7 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt, GB_FW_MGMT_TYPE_LOAD_AND_VALIDATE_FW, &request, sizeof(request), NULL, 0); if (ret) { - ida_simple_remove(&fw_mgmt->id_map, + ida_free(&fw_mgmt->id_map, fw_mgmt->intf_fw_request_id); fw_mgmt->intf_fw_request_id = 0; dev_err(fw_mgmt->parent, @@ -217,7 +217,7 @@ static int fw_mgmt_interface_fw_loaded_operation(struct gb_operation *op) return -ENODEV; } - ida_simple_remove(&fw_mgmt->id_map, fw_mgmt->intf_fw_request_id); + ida_free(&fw_mgmt->id_map, fw_mgmt->intf_fw_request_id); fw_mgmt->intf_fw_request_id = 0; fw_mgmt->intf_fw_status = request->status; fw_mgmt->intf_fw_major = le16_to_cpu(request->major); @@ -327,7 +327,7 @@ static int fw_mgmt_backend_fw_update_operation(struct fw_mgmt *fw_mgmt, GB_FW_MGMT_TYPE_BACKEND_FW_UPDATE, &request, sizeof(request), NULL, 0); if (ret) { - ida_simple_remove(&fw_mgmt->id_map, + ida_free(&fw_mgmt->id_map, fw_mgmt->backend_fw_request_id); fw_mgmt->backend_fw_request_id = 0; dev_err(fw_mgmt->parent, @@ -366,7 +366,7 @@ static int fw_mgmt_backend_fw_updated_operation(struct gb_operation *op) return -ENODEV; } - ida_simple_remove(&fw_mgmt->id_map, fw_mgmt->backend_fw_request_id); + ida_free(&fw_mgmt->id_map, fw_mgmt->backend_fw_request_id); fw_mgmt->backend_fw_request_id = 0; fw_mgmt->backend_fw_status = request->status; @@ -642,7 +642,7 @@ int gb_fw_mgmt_connection_init(struct gb_connection *connection) err_del_cdev: cdev_del(&fw_mgmt->cdev); err_remove_ida: - ida_simple_remove(&fw_mgmt_minors_map, minor); + ida_free(&fw_mgmt_minors_map, minor); err_connection_disable: gb_connection_disable(connection); err_list_del: @@ -666,7 +666,7 @@ void gb_fw_mgmt_connection_exit(struct gb_connection *connection) device_destroy(fw_mgmt_class, fw_mgmt->dev_num); cdev_del(&fw_mgmt->cdev); - ida_simple_remove(&fw_mgmt_minors_map, MINOR(fw_mgmt->dev_num)); + ida_free(&fw_mgmt_minors_map, MINOR(fw_mgmt->dev_num)); /* * Disallow any new ioctl operations on the char device and wait for diff --git a/drivers/staging/greybus/gbphy.c b/drivers/staging/greybus/gbphy.c index 5a5c17a4519b..751d1d580982 100644 --- a/drivers/staging/greybus/gbphy.c +++ b/drivers/staging/greybus/gbphy.c @@ -46,7 +46,7 @@ static void gbphy_dev_release(struct device *dev) { struct gbphy_device *gbphy_dev = to_gbphy_dev(dev); - ida_simple_remove(&gbphy_id, gbphy_dev->id); + ida_free(&gbphy_id, gbphy_dev->id); kfree(gbphy_dev); } @@ -231,7 +231,7 @@ static struct gbphy_device *gb_gbphy_create_dev(struct gb_bundle *bundle, gbphy_dev = kzalloc(sizeof(*gbphy_dev), GFP_KERNEL); if (!gbphy_dev) { - ida_simple_remove(&gbphy_id, id); + ida_free(&gbphy_id, id); return ERR_PTR(-ENOMEM); } diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 2471448ba42a..fca69aff9abf 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -1029,7 +1029,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle, gb->file = debugfs_create_file(name, S_IFREG | 0444, gb_dev.root, gb, &gb_loopback_dbgfs_latency_fops); - gb->id = ida_simple_get(&loopback_ida, 0, 0, GFP_KERNEL); + gb->id = ida_alloc(&loopback_ida, GFP_KERNEL); if (gb->id < 0) { retval = gb->id; goto out_debugfs_remove; @@ -1080,7 +1080,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle, out_connection_disable: gb_connection_disable(connection); out_ida_remove: - ida_simple_remove(&loopback_ida, gb->id); + ida_free(&loopback_ida, gb->id); out_debugfs_remove: debugfs_remove(gb->file); out_connection_destroy: @@ -1122,7 +1122,7 @@ static void gb_loopback_disconnect(struct gb_bundle *bundle) spin_unlock_irqrestore(&gb_dev.lock, flags); device_unregister(gb->dev); - ida_simple_remove(&loopback_ida, gb->id); + ida_free(&loopback_ida, gb->id); gb_connection_destroy(gb->connection); kfree(gb); diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 2a375f407d38..cfaa7ab42ffc 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -178,7 +178,7 @@ static int gb_raw_probe(struct gb_bundle *bundle, raw->connection = connection; greybus_set_drvdata(bundle, raw); - minor = ida_simple_get(&minors, 0, 0, GFP_KERNEL); + minor = ida_alloc(&minors, GFP_KERNEL); if (minor < 0) { retval = minor; goto error_connection_destroy; @@ -211,7 +211,7 @@ static int gb_raw_probe(struct gb_bundle *bundle, gb_connection_disable(connection); error_remove_ida: - ida_simple_remove(&minors, minor); + ida_free(&minors, minor); error_connection_destroy: gb_connection_destroy(connection); @@ -232,7 +232,7 @@ static void gb_raw_disconnect(struct gb_bundle *bundle) device_destroy(raw_class, raw->dev); cdev_del(&raw->cdev); gb_connection_disable(connection); - ida_simple_remove(&minors, MINOR(raw->dev)); + ida_free(&minors, MINOR(raw->dev)); gb_connection_destroy(connection); mutex_lock(&raw->list_lock); diff --git a/drivers/staging/greybus/vibrator.c b/drivers/staging/greybus/vibrator.c index 0e2b188e5ca3..ccc409280fb5 100644 --- a/drivers/staging/greybus/vibrator.c +++ b/drivers/staging/greybus/vibrator.c @@ -154,7 +154,7 @@ static int gb_vibrator_probe(struct gb_bundle *bundle, * there is a "real" device somewhere in the kernel for this, but I * can't find it at the moment... */ - vib->minor = ida_simple_get(&minors, 0, 0, GFP_KERNEL); + vib->minor = ida_alloc(&minors, GFP_KERNEL); if (vib->minor < 0) { retval = vib->minor; goto err_connection_disable; @@ -174,7 +174,7 @@ static int gb_vibrator_probe(struct gb_bundle *bundle, return 0; err_ida_remove: - ida_simple_remove(&minors, vib->minor); + ida_free(&minors, vib->minor); err_connection_disable: gb_connection_disable(connection); err_connection_destroy: @@ -198,7 +198,7 @@ static void gb_vibrator_disconnect(struct gb_bundle *bundle) turn_off(vib); device_unregister(vib->dev); - ida_simple_remove(&minors, vib->minor); + ida_free(&minors, vib->minor); gb_connection_disable(vib->connection); gb_connection_destroy(vib->connection); kfree(vib); -- 2.25.1

2 years, 6 months

[PATCH v6] staging: greybus: introduce pwm_ops::apply

by Song Chen

Introduce newer .apply function in pwm_ops to replace legacy operations including enable, disable, config and set_polarity. This guarantees atomic changes of the pwm controller configuration. Signed-off-by: Song Chen <chensong_2000(a)189.cn> --- v2: 1, define duty_cycle and period as u64 in gb_pwm_config_operation. 2, define duty and period as u64 in gb_pwm_config_request. 3, disable before configuring duty and period if the eventual goal is a disabled state. v3: Regarding duty_cycle and period, I read more discussion in this thread, min, warn or -EINVAL, seems no perfect way acceptable for everyone. How about we limit their value to INT_MAX and throw a warning at the same time when they are wrong? v4: 1, explain why legacy operations are replaced. 2, cap the value of period and duty to U32_MAX. v5: 1, revise commit message. v6: 1, revise commit message. 2, explain why capping the value of period and duty to U32_MAX in comment. --- drivers/staging/greybus/pwm.c | 64 ++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c index 891a6a672378..ad20ec24031e 100644 --- a/drivers/staging/greybus/pwm.c +++ b/drivers/staging/greybus/pwm.c @@ -204,43 +204,59 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); } -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) { + int err; + bool enabled = pwm->state.enabled; + u64 period = state->period; + u64 duty_cycle = state->duty_cycle; struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); -}; + /* Set polarity */ + if (state->polarity != pwm->state.polarity) { + if (enabled) { + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + enabled = false; + } + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); + if (err) + return err; + } -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, - enum pwm_polarity polarity) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (!state->enabled) { + if (enabled) + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + return 0; + } - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); -}; + /* + * Set period and duty cycle + * + * PWM privodes 64-bit period and duty_cycle, but greybus only accepts + * 32-bit, so their values have to be limited to U32_MAX. + */ + if (period > U32_MAX) + period = U32_MAX; -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (duty_cycle > period) + duty_cycle = period; - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); -}; + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_cycle, period); + if (err) + return err; -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + /* enable/disable */ + if (!enabled) + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); - gb_pwm_disable_operation(pwmc, pwm->hwpwm); -}; + return 0; +} static const struct pwm_ops gb_pwm_ops = { .request = gb_pwm_request, .free = gb_pwm_free, - .config = gb_pwm_config, - .set_polarity = gb_pwm_set_polarity, - .enable = gb_pwm_enable, - .disable = gb_pwm_disable, + .apply = gb_pwm_apply, .owner = THIS_MODULE, }; -- 2.25.1

2 years, 6 months

[PATCH v2] staging: greybus: audio: fix loop cursor use after iteration

by Jared Kangas

gbaudio_dapm_free_controls() iterates over widgets using the list_for_each_entry*() family of macros from <linux/list.h>, which leaves the loop cursor pointing to a meaningless structure if it completes a traversal of the list. The cursor was set to NULL at the end of the loop body, but would be overwritten by the final loop cursor update. Because of this behavior, the widget could be non-null after the loop even if the widget wasn't found, and the cleanup logic would treat the pointer as a valid widget to free. To fix this, introduce a temporary variable to act as the loop cursor and copy it to a variable that can be accessed after the loop finishes. Due to not removing any list elements, use list_for_each_entry() instead of list_for_each_entry_safe() in the revised loop. This was detected with the help of Coccinelle. Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic audio modules") Cc: stable(a)vger.kernel.org Reviewed-by: Dan Carpenter <dan.carpenter(a)oracle.com> Reviewed-by: Johan Hovold <johan(a)kernel.org> Signed-off-by: Jared Kangas <kangas.jd(a)gmail.com> --- Changes since v1: * Removed safe list iteration as suggested by Johan Hovold <johan(a)kernel.org> * Updated patch changelog to explain the list iteration change * Added tags to changelog based on feedback (Cc:, Fixes:, Reviewed-by:) drivers/staging/greybus/audio_helper.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c index 843760675876..05e91e6bc2a0 100644 --- a/drivers/staging/greybus/audio_helper.c +++ b/drivers/staging/greybus/audio_helper.c @@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, int num) { int i; - struct snd_soc_dapm_widget *w, *next_w; + struct snd_soc_dapm_widget *w, *tmp_w; #ifdef CONFIG_DEBUG_FS struct dentry *parent = dapm->debugfs_dapm; struct dentry *debugfs_w = NULL; @@ -124,13 +124,13 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, mutex_lock(&dapm->card->dapm_mutex); for (i = 0; i < num; i++) { /* below logic can be optimized to identify widget pointer */ - list_for_each_entry_safe(w, next_w, &dapm->card->widgets, - list) { - if (w->dapm != dapm) - continue; - if (!strcmp(w->name, widget->name)) + w = NULL; + list_for_each_entry(tmp_w, &dapm->card->widgets, list) { + if (tmp_w->dapm == dapm && + !strcmp(tmp_w->name, widget->name)) { + w = tmp_w; break; - w = NULL; + } } if (!w) { dev_err(dapm->dev, "%s: widget not found\n", -- 2.34.3

2 years, 6 months

[PATCH] staging: greybus: audio: replace safe list iteration

by Johan Hovold

No entry is being removed from the list when iterating the widget list in gbaudio_dapm_free_controls() so there's no need to use list_for_each_entry_safe(). Signed-off-by: Johan Hovold <johan(a)kernel.org> --- drivers/staging/greybus/audio_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c index 07461a5d97c7..05e91e6bc2a0 100644 --- a/drivers/staging/greybus/audio_helper.c +++ b/drivers/staging/greybus/audio_helper.c @@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, int num) { int i; - struct snd_soc_dapm_widget *w, *next_w, *tmp_w; + struct snd_soc_dapm_widget *w, *tmp_w; #ifdef CONFIG_DEBUG_FS struct dentry *parent = dapm->debugfs_dapm; struct dentry *debugfs_w = NULL; @@ -125,8 +125,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, for (i = 0; i < num; i++) { /* below logic can be optimized to identify widget pointer */ w = NULL; - list_for_each_entry_safe(tmp_w, next_w, &dapm->card->widgets, - list) { + list_for_each_entry(tmp_w, &dapm->card->widgets, list) { if (tmp_w->dapm == dapm && !strcmp(tmp_w->name, widget->name)) { w = tmp_w; -- 2.35.1

2 years, 6 months

[PATCH] staging: greybus: audio: fix loop cursor use after iteration

by Jared Kangas

gbaudio_dapm_free_controls() iterates over widgets using list_for_each_entry_safe(), which leaves the loop cursor pointing to a meaningless structure if it completes a traversal of the list. The cursor was set to NULL at the end of the loop body, but would be overwritten by the final loop cursor update. Because of this behavior, the widget could be non-null after the loop even if the widget wasn't found, and the cleanup logic would treat the pointer as a valid widget to free. To fix this, introduce a temporary variable to act as the loop cursor and copy it to a variable that can be accessed after the loop finishes. This was detected with the help of Coccinelle. Signed-off-by: Jared Kangas <kangas.jd(a)gmail.com> --- drivers/staging/greybus/audio_helper.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c index 843760675876..07461a5d97c7 100644 --- a/drivers/staging/greybus/audio_helper.c +++ b/drivers/staging/greybus/audio_helper.c @@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, int num) { int i; - struct snd_soc_dapm_widget *w, *next_w; + struct snd_soc_dapm_widget *w, *next_w, *tmp_w; #ifdef CONFIG_DEBUG_FS struct dentry *parent = dapm->debugfs_dapm; struct dentry *debugfs_w = NULL; @@ -124,13 +124,14 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm, mutex_lock(&dapm->card->dapm_mutex); for (i = 0; i < num; i++) { /* below logic can be optimized to identify widget pointer */ - list_for_each_entry_safe(w, next_w, &dapm->card->widgets, + w = NULL; + list_for_each_entry_safe(tmp_w, next_w, &dapm->card->widgets, list) { - if (w->dapm != dapm) - continue; - if (!strcmp(w->name, widget->name)) + if (tmp_w->dapm == dapm && + !strcmp(tmp_w->name, widget->name)) { + w = tmp_w; break; - w = NULL; + } } if (!w) { dev_err(dapm->dev, "%s: widget not found\n", -- 2.34.3

2 years, 6 months

[PATCH] staging: greybus: loopback: fix a spelling error.

by Steve Lemuel

asynchronus -> asynchronous Signed-off-by: Steve Lemuel <wlemuel(a)hotmail.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 2471448ba42a..1a61fce98056 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -870,7 +870,7 @@ static int gb_loopback_fn(void *data) if (gb->send_count == gb->iteration_max) { mutex_unlock(&gb->mutex); - /* Wait for synchronous and asynchronus completion */ + /* Wait for synchronous and asynchronous completion */ gb_loopback_async_wait_all(gb); /* Mark complete unless user-space has poked us */ -- 2.36.1

2 years, 6 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev June 2022