linux-kernel - Re: [RFC PATCH 10/11] drivers/misc/vmw_vmci: convert num guest devices counter to counter

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]

Date:   Wed, 23 Sep 2020 12:29:57 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     arnd@...db.de, keescook@...omium.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 10/11] drivers/misc/vmw_vmci: convert num guest
 devices counter to counter_atomic

On Tue, Sep 22, 2020 at 07:43:39PM -0600, Shuah Khan wrote:
> counter_atomic is introduced to be used when a variable is used as
> a simple counter and doesn't guard object lifetimes. This clearly
> differentiates atomic_t usages that guard object lifetimes.
> 
> counter_atomic variables will wrap around to 0 when it overflows and
> should not be used to guard resource lifetimes, device usage and
> open counts that control state changes, and pm states.
> 
> atomic_t variable used to count number of vmci guest devices is used
> as just as counter and it doesn't control object lifetimes or state
> management. Overflow doesn't appear to be problem for this use.
> 
> Convert it to use counter_atomic.
> 
> This conversion doesn't change the oveflow wrap around behavior.
> 
> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
> ---
>  drivers/misc/vmw_vmci/vmci_guest.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
> index cc8eeb361fcd..2c21448af730 100644
> --- a/drivers/misc/vmw_vmci/vmci_guest.c
> +++ b/drivers/misc/vmw_vmci/vmci_guest.c
> @@ -20,6 +20,7 @@
>  #include <linux/smp.h>
>  #include <linux/io.h>
>  #include <linux/vmalloc.h>
> +#include <linux/counters.h>
>  
>  #include "vmci_datagram.h"
>  #include "vmci_doorbell.h"
> @@ -68,11 +69,11 @@ struct pci_dev *vmci_pdev;
>  static struct vmci_guest_device *vmci_dev_g;
>  static DEFINE_SPINLOCK(vmci_dev_spinlock);
>  
> -static atomic_t vmci_num_guest_devices = ATOMIC_INIT(0);
> +static struct counter_atomic vmci_num_guest_devices = COUNTER_ATOMIC_INIT(0);
>  
>  bool vmci_guest_code_active(void)
>  {
> -	return atomic_read(&vmci_num_guest_devices) != 0;
> +	return counter_atomic_read(&vmci_num_guest_devices) != 0;
>  }
>  
>  u32 vmci_get_vm_context_id(void)
> @@ -624,7 +625,7 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
>  
>  	dev_dbg(&pdev->dev, "Registered device\n");
>  
> -	atomic_inc(&vmci_num_guest_devices);
> +	counter_atomic_inc(&vmci_num_guest_devices);
>  
>  	/* Enable specific interrupt bits. */
>  	cmd = VMCI_IMR_DATAGRAM;
> @@ -684,7 +685,7 @@ static void vmci_guest_remove_device(struct pci_dev *pdev)
>  
>  	dev_dbg(&pdev->dev, "Removing device\n");
>  
> -	atomic_dec(&vmci_num_guest_devices);
> +	counter_atomic_dec(&vmci_num_guest_devices);
>  
>  	vmci_qp_guest_endpoints_exit();
>  

While this conversion looks fine to me, wow, the code does not really
seem to be doing the right thing.  So that's something to fix up
independantly of this change, not your fault at all :)

Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>