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: <YFDg2gcNSEbf5Qle@kroah.com>
Date:   Tue, 16 Mar 2021 17:46:18 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Mihai Carabas <mihai.carabas@...cle.com>
Cc:     linux-kernel@...r.kernel.org, arnd@...db.de,
        bobo.shaobowang@...wei.com, rdunlap@...radead.org
Subject: Re: [PATCH v5 2/3] misc/pvpanic: probe multiple instances

On Tue, Mar 16, 2021 at 02:20:28PM +0200, Mihai Carabas wrote:
> Create the mecahism that allows multiple pvpanic instances to call
> pvpanic_probe and receive panic events. A global list will retain all the
> mapped addresses where to write panic events.
> 
> Signed-off-by: Mihai Carabas <mihai.carabas@...cle.com>
> ---
>  drivers/misc/pvpanic/pvpanic-mmio.c |  11 ++--
>  drivers/misc/pvpanic/pvpanic.c      | 105 +++++++++++++++++++++++++++++-------
>  drivers/misc/pvpanic/pvpanic.h      |   6 +--
>  3 files changed, 92 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c
> index 801d892..62ab8d7 100644
> --- a/drivers/misc/pvpanic/pvpanic-mmio.c
> +++ b/drivers/misc/pvpanic/pvpanic-mmio.c
> @@ -54,7 +54,7 @@ static ssize_t events_store(struct device *dev,  struct device_attribute *attr,
>  
>  	events = tmp;
>  
> -	pvpanic_set_events(events);
> +	pvpanic_set_events(base, events);
>  
>  	return count;
>  
> @@ -72,7 +72,6 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
> -	void __iomem *base;
>  
>  	res = platform_get_mem_or_io(pdev, 0);
>  	if (!res)
> @@ -97,17 +96,13 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
>  	capability &= ioread8(base);
>  	events = capability;
>  
> -	pvpanic_probe(base, capability);
> -
> -	return 0;
> +	return pvpanic_probe(base, capability);
>  }
>  
>  static int pvpanic_mmio_remove(struct platform_device *pdev)
>  {
>  
> -	pvpanic_remove();
> -
> -	return 0;
> +	return pvpanic_remove(base);
>  }
>  
>  static const struct of_device_id pvpanic_mmio_match[] = {
> diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> index 11d029c..73a82d9 100644
> --- a/drivers/misc/pvpanic/pvpanic.c
> +++ b/drivers/misc/pvpanic/pvpanic.c
> @@ -14,20 +14,35 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/types.h>
> +#include <linux/cdev.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
>  
>  #include <uapi/misc/pvpanic.h>
>  
>  #include "pvpanic.h"
>  
> -static void __iomem *base;
> -static unsigned int capability;
> -static unsigned int events;
> +struct pvpanic_instance {
> +	void __iomem *base;
> +	unsigned int capability;
> +	unsigned int events;
> +	struct list_head list;
> +};
> +
> +struct list_head pvpanic_list;
> +spinlock_t pvpanic_lock;
>  
>  static void
>  pvpanic_send_event(unsigned int event)
>  {
> -	if (event & capability & events)
> -		iowrite8(event, base);
> +	struct pvpanic_instance *pi_cur;
> +
> +	spin_lock(&pvpanic_lock);
> +	list_for_each_entry(pi_cur, &pvpanic_list, list) {
> +		if (event & pi_cur->capability & pi_cur->events)
> +			iowrite8(event, pi_cur->base);
> +	}
> +	spin_unlock(&pvpanic_lock);
>  }
>  
>  static int
> @@ -49,29 +64,81 @@
>  	.priority = 1, /* let this called before broken drm_fb_helper */
>  };
>  
> -void pvpanic_probe(void __iomem *pbase, unsigned int dev_cap)
> +int pvpanic_probe(void __iomem *pbase, unsigned int dev_cap)
>  {
> -	base = pbase;
> -	capability = dev_cap;
> -	events = capability;
> +	struct pvpanic_instance *pi;
> +
> +	if (!pbase)
> +		return -EINVAL;
> +
> +	pi = kmalloc(sizeof(*pi), GFP_ATOMIC);
> +	if (!pi)
> +		return -ENOMEM;
> +
> +	pi->base = pbase;
> +	pi->capability = dev_cap;
> +	pi->events = dev_cap;
> +	spin_lock(&pvpanic_lock);
> +	list_add(&pi->list, &pvpanic_list);
> +	spin_unlock(&pvpanic_lock);
>  
> -	if (capability)
> -		atomic_notifier_chain_register(&panic_notifier_list,
> -					       &pvpanic_panic_nb);
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(pvpanic_probe);
>  
> -void pvpanic_remove(void)
> +int pvpanic_remove(void __iomem *pbase)
>  {
> -	if (capability)
> -		atomic_notifier_chain_unregister(&panic_notifier_list,
> -						 &pvpanic_panic_nb);
> -	base = NULL;
> +	struct pvpanic_instance *pi_cur, *pi_next;
> +
> +	if (!pbase)
> +		return -EINVAL;
> +
> +	spin_lock(&pvpanic_lock);
> +	list_for_each_entry_safe(pi_cur, pi_next, &pvpanic_list, list) {
> +		if (pi_cur->base == pbase) {
> +			list_del(&pi_cur->list);
> +			kfree(pi_cur);
> +			break;
> +		}
> +	}
> +	spin_unlock(&pvpanic_lock);
> +
> +	return 0;
>  }

As "remove" can not fail, this should just stay a void function.  People
are working to remove the int return value from remove calls in the
kernel right now, please do not add new ones if at all possible.

thanks,

greg k-h

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ