linux-kernel - Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit is cleared

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:   Thu, 17 Nov 2022 14:50:55 +0100
From:   Jan Beulich <jbeulich@...e.com>
To:     Marek Marczykowski-Górecki 
        <marmarek@...isiblethingslab.com>
Cc:     Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@...ts.xenproject.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit
 is cleared

On 17.11.2022 12:41, Marek Marczykowski-Górecki wrote:
> --- a/drivers/xen/xen-pciback/conf_space.c
> +++ b/drivers/xen/xen-pciback/conf_space.c
> @@ -313,7 +313,7 @@ int xen_pcibk_get_interrupt_type(struct pci_dev *dev)
>  				&val);
>  		if (err)
>  			return err;
> -		if (val & PCI_MSIX_FLAGS_ENABLE)
> +		if (val & PCI_MSIX_FLAGS_ENABLE && !(val & PCI_MSIX_FLAGS_MASKALL))
>  			ret |= INTERRUPT_TYPE_MSIX;
>  	}
>  	return ret ?: INTERRUPT_TYPE_NONE;

Considering Andrew's reply, maybe it wasn't a good suggestion to change
the code here. If, however, you/we decide to keep the change, then
please add another pair of parentheses around the operands of the left
hand &.

If the change was to be dropped again, I think ...

> --- a/drivers/xen/xen-pciback/conf_space_capability.c
> +++ b/drivers/xen/xen-pciback/conf_space_capability.c
> @@ -242,6 +242,10 @@ static int msi_msix_flags_write(struct pci_dev *dev, int offset, u16 new_value,
>  		if (int_type == INTERRUPT_TYPE_NONE ||
>  		    int_type == field_config->int_type)
>  			goto write;
> +		if (int_type == INTERRUPT_TYPE_INTX &&

... this would need extending to also cover the INTX|MSIX case (i.e.
if a second such write made it here when the enable+maskall bits are
already set).

Jan

> +		    field_config->int_type == INTERRUPT_TYPE_MSIX &&
> +		    (new_value & PCI_MSIX_FLAGS_MASKALL))
> +			goto write;
>  		return PCIBIOS_SET_FAILED;
>  	}
>