linux-kernel - Re: [PATCH v2 1/5] misc: pci_endpoint_test: Fix the return value of IOCTL

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, 24 Aug 2022 14:43:18 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     kishon@...com, lpieralisi@...nel.org, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, mie@...l.co.jp, kw@...ux.com,
        stable@...r.kernel.org
Subject: Re: [PATCH v2 1/5] misc: pci_endpoint_test: Fix the return value of
 IOCTL

On Wed, Aug 24, 2022 at 06:00:06PM +0530, Manivannan Sadhasivam wrote:
> IOCTLs are supposed to return 0 for success and negative error codes for
> failure. Currently, this driver is returning 0 for failure and 1 for
> success, that's not correct. Hence, fix it!
> 
> Cc: stable@...r.kernel.org #5.10
> Fixes: 2c156ac71c6b ("misc: Add host side PCI driver for PCI test function device")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

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

> ---
>  drivers/misc/pci_endpoint_test.c | 163 ++++++++++++++-----------------
>  1 file changed, 76 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 8f786a225dcf..a7d8ae9730f6 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -174,13 +174,12 @@ static void pci_endpoint_test_free_irq_vectors(struct pci_endpoint_test *test)
>  	test->irq_type = IRQ_TYPE_UNDEFINED;
>  }
>  
> -static bool pci_endpoint_test_alloc_irq_vectors(struct pci_endpoint_test *test,
> +static int pci_endpoint_test_alloc_irq_vectors(struct pci_endpoint_test *test,
>  						int type)
>  {
> -	int irq = -1;
> +	int irq = -ENOSPC;

No need to set this if you:

>  	struct pci_dev *pdev = test->pdev;
>  	struct device *dev = &pdev->dev;
> -	bool res = true;
>  
>  	switch (type) {
>  	case IRQ_TYPE_LEGACY:
> @@ -202,15 +201,16 @@ static bool pci_endpoint_test_alloc_irq_vectors(struct pci_endpoint_test *test,
>  		dev_err(dev, "Invalid IRQ type selected\n");

This should now return -EINVAL;


>  	}
>  
> +	test->irq_type = type;

Again, do not make a change to the kernel state if there is an error
above.  That's wrong to do, and yes, the current code is incorrect,
don't keep that bug here as well when it's so easy to fix up
automatically.


I stopped reviewing here...

thanks,

greg k-h