linux-kernel - Re: [Xen-devel] [PATCH 1/2] xen-pciback: Fix error return in bar_write() and rom

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]

Message-ID: <20181203123519.l34a4yx26qle3whb@mac>
Date:   Mon, 3 Dec 2018 13:35:19 +0100
From:   Roger Pau Monné <roger.pau@...rix.com>
To:     Marek Marczykowski-Górecki 
        <marmarek@...isiblethingslab.com>
CC:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Dwayne Litzenberger <dlitz@...tz.net>,
        <xen-devel@...ts.xenproject.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH 1/2] xen-pciback: Fix error return in
 bar_write() and rom_write()

On Sun, Dec 02, 2018 at 06:47:32PM +0100, Marek Marczykowski-Górecki wrote:
> From: Dwayne Litzenberger <dlitz@...tz.net>

I think this requires some description. At least a note that the
function is not altered, just errors from pci reads/writes are no
longer ignored.

> Signed-off-by: Dwayne Litzenberger <dlitz@...tz.net>
> ---
>  drivers/xen/xen-pciback/conf_space_header.c | 24 ++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/xen/xen-pciback/conf_space_header.c b/drivers/xen/xen-pciback/conf_space_header.c
> index 10ae24b..697d0a8 100644
> --- a/drivers/xen/xen-pciback/conf_space_header.c
> +++ b/drivers/xen/xen-pciback/conf_space_header.c
> @@ -135,6 +135,7 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
>  
>  static int rom_write(struct pci_dev *dev, int offset, u32 value, void *data)
>  {
> +	int err = 0;
>  	struct pci_bar_info *bar = data;
>  
>  	if (unlikely(!bar)) {
> @@ -150,17 +151,22 @@ static int rom_write(struct pci_dev *dev, int offset, u32 value, void *data)
>  		bar->which = 1;
>  	else {
>  		u32 tmpval;
> -		pci_read_config_dword(dev, offset, &tmpval);
> +		err = pci_read_config_dword(dev, offset, &tmpval);
> +		if (err)
> +			goto out;

I don't think you need the out label, you could just return err.
Adding the label doesn't help in any way IMO, just makes the function
one line longer for no reason.

Same for the out label added to bar_write.

Thanks, Roger.