[PATCH] x86/msi: Don't dereference pdev before its NULL check

[PATCH] x86/msi: Don't dereference pdev before its NULL check

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/msi.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index fa95048..160167c 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -1345,7 +1345,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
     struct msi_desc *entry, *tmp;
     struct irq_desc *desc;
     struct msi_msg msg;
-    u8 slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
+    u8 slot, func;
     unsigned int type = 0, pos = 0;
     u16 control = 0;
 
@@ -1357,6 +1357,9 @@ int pci_restore_msi_state(struct pci_dev *pdev)
     if ( !pdev )
         return -EINVAL;
 
+    slot = PCI_SLOT(pdev->devfn);
+    func = PCI_FUNC(pdev->devfn);
+
     ret = xsm_resource_setup_pci(XSM_PRIV,
                                 (pdev->seg << 16) | (pdev->bus << 8) |
                                 pdev->devfn);
-- 
1.7.10.4

Re: [PATCH] x86/msi: Don't dereference pdev before its NULL check

[Jan Beulich]

>>> On 23.07.15 at 12:54, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -1345,7 +1345,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>      struct msi_desc *entry, *tmp;
>      struct irq_desc *desc;
>      struct msi_msg msg;
> -    u8 slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
> +    u8 slot, func;
>      unsigned int type = 0, pos = 0;
>      u16 control = 0;
>  
> @@ -1357,6 +1357,9 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>      if ( !pdev )
>          return -EINVAL;
>  
> +    slot = PCI_SLOT(pdev->devfn);
> +    func = PCI_FUNC(pdev->devfn);

I think this is the wrong way round - see the alternative patch just
sent.

Btw, how did you spot this so quickly now but not during review?
Any tool helping with that?

Jan

Re: [PATCH] x86/msi: Don't dereference pdev before its NULL check

[Andrew Cooper]

On 23/07/15 12:45, Jan Beulich wrote:
>>>> On 23.07.15 at 12:54, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/msi.c
>> +++ b/xen/arch/x86/msi.c
>> @@ -1345,7 +1345,7 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>>      struct msi_desc *entry, *tmp;
>>      struct irq_desc *desc;
>>      struct msi_msg msg;
>> -    u8 slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
>> +    u8 slot, func;
>>      unsigned int type = 0, pos = 0;
>>      u16 control = 0;
>>  
>> @@ -1357,6 +1357,9 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>>      if ( !pdev )
>>          return -EINVAL;
>>  
>> +    slot = PCI_SLOT(pdev->devfn);
>> +    func = PCI_FUNC(pdev->devfn);
> I think this is the wrong way round - see the alternative patch just
> sent.
>
> Btw, how did you spot this so quickly now but not during review?
> Any tool helping with that?

Ah - I accidentally neglected to say that it was Coverity.  It is hooked
automatically into all XenServer builds, and I am being proactive
keeping my upstream branch up-to-date, given the volume of code going in
at the moment.

~Andrew