| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 21 Jan 2021 00:27:43 -0800
From: Saravana Kannan <saravanak@...gle.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>
Cc: Marek Szyprowski <m.szyprowski@...sung.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Marc Zyngier <maz@...nel.org>,
Android Kernel Team <kernel-team@...roid.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [TEST PATCH v1] driver: core: Make fw_devlink=on more forgiving
On Thu, Jan 21, 2021 at 12:22 AM Saravana Kannan <saravanak@...gle.com> wrote:
>
> This patch is for test purposes only and pretty experimental. Code might
> not be optimized, clean, formatted properly, etc.
>
> Please review it only for functional bugs like locking bugs, wrong
> logic, etc.
>
> It's basically trying to figure out which devices will never probe and
> ignore them. Might not always work.
>
> Marek, Geert, Marc,
>
> Can you please try this patch INSTEAD of the other workarounds we found?
Oh and can you please also try with the CONFIG_MODULES enabled vs
disabled? Or have it disabled but fix the patch so the condition
always evaluates to true.
Thanks,
Saravana
>
> Jon, Michael,
>
> I'm explicitly not including you in the "To" because this patch won't
> work for your issues.
>
> Cc: Marek Szyprowski <m.szyprowski@...sung.com>
> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: Marc Zyngier <maz@...nel.org>
> Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> ---
> drivers/base/base.h | 3 ++
> drivers/base/core.c | 117 +++++++++++++++++++++++++++++++++++++++++++-
> drivers/base/dd.c | 24 +++++++++
> 3 files changed, 142 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index f5600a83124f..8d5fd95fa147 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -106,6 +106,9 @@ struct device_private {
> #define to_device_private_class(obj) \
> container_of(obj, struct device_private, knode_class)
>
> +bool fw_devlink_is_permissive(void);
> +bool fw_devlink_unblock_probe(struct device *dev);
> +
> /* initialisation functions */
> extern int devices_init(void);
> extern int buses_init(void);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index e61e62b624ce..8528704bbb40 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -49,7 +49,6 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup);
> static LIST_HEAD(deferred_sync);
> static unsigned int defer_sync_state_count = 1;
> static DEFINE_MUTEX(fwnode_link_lock);
> -static bool fw_devlink_is_permissive(void);
>
> /**
> * fwnode_link_add - Create a link between two fwnode_handles.
> @@ -1481,7 +1480,7 @@ u32 fw_devlink_get_flags(void)
> return fw_devlink_flags;
> }
>
> -static bool fw_devlink_is_permissive(void)
> +bool fw_devlink_is_permissive(void)
> {
> return fw_devlink_flags == FW_DEVLINK_FLAGS_PERMISSIVE;
> }
> @@ -1552,6 +1551,120 @@ static int fw_devlink_relax_cycle(struct device *con, void *sup)
> return ret;
> }
>
> +static int __device_links_suppliers_available(struct device *dev)
> +{
> + struct device_link *link;
> + int ret = 0;
> +
> + if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) &&
> + !fw_devlink_is_permissive()) {
> + return -EPROBE_DEFER;
> + }
> +
> + list_for_each_entry(link, &dev->links.suppliers, c_node) {
> + if (!(link->flags & DL_FLAG_MANAGED))
> + continue;
> +
> + if (link->status != DL_STATE_AVAILABLE &&
> + !(link->flags & DL_FLAG_SYNC_STATE_ONLY)) {
> + ret = -EPROBE_DEFER;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +bool fw_devlink_unblock_probe(struct device *dev)
> +{
> + struct fwnode_link *link, *tmp;
> + struct device_link *dev_link, *dev_ln;
> + struct fwnode_handle *fwnode = dev->fwnode;
> + bool unblocked = false;
> +
> + if (!fw_devlink_get_flags() || fw_devlink_is_permissive())
> + return false;
> +
> + if (!fwnode)
> + return false;
> +
> + mutex_lock(&fwnode_link_lock);
> +
> + /* Delete questionable fwnode links */
> + list_for_each_entry_safe(link, tmp, &fwnode->suppliers, c_hook) {
> + struct device *par_dev;
> + struct fwnode_handle *par;
> + bool bound;
> +
> + /*
> + * Walk up fwnode tree of supplier till we find a parent device
> + * that has been added or a parent fwnode that has fwnode links
> + * (this is a firmware node that is expected to be added as a
> + * device in the future).
> + */
> + par = fwnode_get_parent(link->supplier);
> + while (par && list_empty(&par->suppliers) && !par->dev)
> + par = fwnode_get_next_parent(par);
> +
> + /* Supplier is waiting on parent device to be added. */
> + if (par && !par->dev) {
> + fwnode_handle_put(par);
> + continue;
> + }
> +
> + if (par && par->dev) {
> + par_dev = get_dev_from_fwnode(fwnode);
> + device_lock(par_dev);
> + bound = device_is_bound(par_dev);
> + device_unlock(par_dev);
> + put_device(par_dev);
> +
> + /* Supplier is waiting on parent device to be bound. */
> + if (!bound)
> + continue;
> + }
> +
> + /*
> + * Supplier has no parent or the immediate parent device has
> + * been bound to a device. It should have been added by now.
> + * So, this link is spurious. Delete it.
> + */
> + dev_info(dev, "Deleting fwnode link to %pfwP\n",
> + link->supplier);
> + list_del(&link->s_hook);
> + list_del(&link->c_hook);
> + kfree(link);
> + unblocked = true;
> + }
> +
> + if (IS_ENABLED(CONFIG_MODULES))
> + goto out;
> +
> + device_links_write_lock();
> +
> + list_for_each_entry_safe(dev_link, dev_ln, &dev->links.suppliers,
> + c_node) {
> + if (!(dev_link->flags & DL_FLAG_INFERRED) ||
> + dev_link->flags & DL_FLAG_SYNC_STATE_ONLY ||
> + dev_link->status != DL_STATE_DORMANT)
> + continue;
> +
> + /* This supplier should have probed by now. */
> + if (!__device_links_suppliers_available(dev_link->supplier)) {
> + dev_info(dev, "Deleting dev link to %s\n",
> + dev_name(dev_link->supplier));
> + device_link_drop_managed(dev_link);
> + unblocked = true;
> + }
> + }
> +
> + device_links_write_unlock();
> +
> +out:
> + mutex_unlock(&fwnode_link_lock);
> + return unblocked;
> +}
> +
> /**
> * fw_devlink_create_devlink - Create a device link from a consumer to fwnode
> * @con - Consumer device for the device link
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 2f32f38a11ed..d4ccd2a2b6a4 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -301,6 +301,25 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
> }
> static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
>
> +static bool deferred_probe_fw_devlink_unblock(void)
> +{
> + struct device *dev;
> + struct device_private *private;
> + bool unblocked = false;
> +
> + if (!fw_devlink_get_flags() || fw_devlink_is_permissive())
> + return false;
> +
> + mutex_lock(&deferred_probe_mutex);
> + list_for_each_entry(private, &deferred_probe_pending_list, deferred_probe) {
> + dev = private->device;
> + unblocked |= fw_devlink_unblock_probe(dev);
> + }
> + mutex_unlock(&deferred_probe_mutex);
> +
> + return unblocked;
> +}
> +
> /**
> * deferred_probe_initcall() - Enable probing of deferred devices
> *
> @@ -317,6 +336,11 @@ static int deferred_probe_initcall(void)
> driver_deferred_probe_trigger();
> /* Sort as many dependencies as possible before exiting initcalls */
> flush_work(&deferred_probe_work);
> +
> + while (deferred_probe_fw_devlink_unblock()) {
> + driver_deferred_probe_trigger();
> + flush_work(&deferred_probe_work);
> + }
> initcalls_done = true;
>
> /*
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
Powered by blists - more mailing lists