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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y+4kwf3SRq5NBzBz@unreal>
Date:   Thu, 16 Feb 2023 14:42:41 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     "Temerkhanov, Sergey" <sergey.temerkhanov@...el.com>
Cc:     netdev@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH net-next] auxiliary: Implement refcounting

+ Greg KH

On Thu, Feb 16, 2023 at 01:16:21PM +0100, Temerkhanov, Sergey wrote:
> From: Sergey Temerkhanov <sergey.temerkhanov@...el.com>
> 
> Implement reference counting to make it possible to synchronize
> deinitialization and removal of interfaces published via aux bus
> with the client modules.
> Reference counting can be used in both sleeping and non-sleeping
> contexts so this approach is intended to replace device_lock()
> (mutex acquisition) with an additional lock on top of it
> which is not always possible to take in client code.

I want to see this patch as part of your client code series.
It is unclear why same aux device is used from different drivers.

Otherwise, this whole refcnt is useless and just hides bugs in driver
which accesses released devices.

> 
> Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@...el.com>
> ---
>  drivers/base/auxiliary.c      | 18 ++++++++++++++++++
>  include/linux/auxiliary_bus.h | 34 +++++++++++++++++++++++++---------
>  2 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index 8c5e65930617..082b3ebd143d 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -287,10 +287,28 @@ int auxiliary_device_init(struct auxiliary_device *auxdev)
>  
>  	dev->bus = &auxiliary_bus_type;
>  	device_initialize(&auxdev->dev);
> +	init_waitqueue_head(&auxdev->wq_head);
> +	refcount_set(&auxdev->refcnt, 1);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(auxiliary_device_init);
>  
> +void auxiliary_device_uninit(struct auxiliary_device *auxdev)
> +{
> +	wait_event_interruptible(auxdev->wq_head,
> +				 refcount_dec_if_one(&auxdev->refcnt));
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_device_uninit);
> +
> +void auxiliary_device_delete(struct auxiliary_device *auxdev)
> +{
> +	WARN_ON(refcount_read(&auxdev->refcnt));
> +
> +	device_del(&auxdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_device_delete);
> +
>  /**
>   * __auxiliary_device_add - add an auxiliary bus device
>   * @auxdev: auxiliary bus device to add to the bus
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index de21d9d24a95..0610ccee320e 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -10,6 +10,8 @@
>  
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/wait.h>
> +#include <linux/refcount.h>
>  
>  /**
>   * DOC: DEVICE_LIFESPAN
> @@ -137,7 +139,9 @@
>   */
>  struct auxiliary_device {
>  	struct device dev;
> +	refcount_t refcnt;
>  	const char *name;
> +	struct wait_queue_head wq_head;
>  	u32 id;
>  };
>  
> @@ -198,6 +202,25 @@ static inline void auxiliary_set_drvdata(struct auxiliary_device *auxdev, void *
>  	dev_set_drvdata(&auxdev->dev, data);
>  }
>  
> +static inline bool __must_check
> +auxiliary_device_get(struct auxiliary_device *adev)
> +{
> +	if (!adev)
> +		return false;

Please don't check for validity of adev, it is caller's job.

> +
> +	return refcount_inc_not_zero(&adev->refcnt);
> +}
> +
> +static inline void auxiliary_device_put(struct auxiliary_device *adev)
> +{
> +	if (!adev)
> +		return;

Same.

Thanks

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ