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

Manivannan Sadhasivam
2022-08-24T14:26:01Z

On Wed, Aug 24, 2022 at 02:43:18PM +0200, Greg KH wrote:
> 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@vger.kernel.org #5.10
> > Fixes: 2c156ac71c6b ("misc: Add host side PCI driver for PCI test function device")
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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;
>

ack

>
> > }
> >
> > + 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.
>

Okay. Will fix it in this patch itself.

Thanks,
Mani

>
> I stopped reviewing here...
>
> thanks,
>
> greg k-h

--
மணிவண்ணன் சதாசிவம்