linux-kernel - Re: [PATCH 5/7] scsi: qla4xxx: shut up warning for rd_reg

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] [day] [month] [year] [list]

Date:	Mon, 1 Feb 2016 08:17:48 +0000
From:	Nilesh Javali <nilesh.javali@...gic.com>
To:	Arnd Bergmann <arnd@...db.de>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	"James.Bottomley@...senPartnership.com" 
	<James.Bottomley@...senPartnership.com>
CC:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	"James E.J. Bottomley" <JBottomley@...n.com>,
	Dept-Eng QLogic Storage Upstream 
	<QLogic-Storage-Upstream@...gic.com>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/7] scsi: qla4xxx: shut up warning for rd_reg_indirect



On 27/01/16 9:27 PM, "Arnd Bergmann" <arnd@...db.de> wrote:

>The qla4_83xx_rd_reg_indirect() function can fail when it is unable
>to read a register, but not all callers check its return value before
>using the register data, and gcc correctly warns about this:
>
>qla4xxx/ql4_83xx.c: In function 'qla4_83xx_process_reset_template':
>qla4xxx/ql4_83xx.c:1073:36: warning: 'value' may be used uninitialized in
>this function
>     ha->reset_tmplt.array[index++] = value;
>                                    ^
>qla4xxx/ql4_83xx.c:1050:11: note: 'value' was declared here
>  uint32_t value;
>           ^
>qla4xxx/ql4_83xx.c:902:8: warning: 'value' may be used uninitialized in
>this function
>  value &= p_rmw_hdr->test_mask;
>        ^
>qla4xxx/ql4_83xx.c:895:11: note: 'value' was declared here
>  uint32_t value;
>           ^
>In file included from ../include/linux/io.h:25:0,
>                 from ../include/linux/pci.h:31,
>                 from ../drivers/scsi/qla4xxx/ql4_def.h:16,
>                 from ../drivers/scsi/qla4xxx/ql4_83xx.c:10:
>asm/io.h:101:2: warning: 'value' may be used uninitialized in this
>function
>  asm volatile("str %1, %0"
>  ^
>qla4xxx/ql4_83xx.c:874:11: note: 'value' was declared here
>  uint32_t value;
>           ^
>
>Unfortunately, I don't see any helpful way to add proper error handling
>for this case, and the failure scenario for rd_reg seems rather obscure,
>so this bails out and makes the rd_reg accessor set the result to
>0xffffffff
>so we at least get a predictable value.
>
>Signed-off-by: Arnd Bergmann <arnd@...db.de>
>---
> drivers/scsi/qla4xxx/ql4_83xx.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/scsi/qla4xxx/ql4_83xx.c
>b/drivers/scsi/qla4xxx/ql4_83xx.c
>index 5d4f8e67fb25..638f72c5ab05 100644
>--- a/drivers/scsi/qla4xxx/ql4_83xx.c
>+++ b/drivers/scsi/qla4xxx/ql4_83xx.c
>@@ -46,11 +46,13 @@ int qla4_83xx_rd_reg_indirect(struct scsi_qla_host
>*ha, uint32_t addr,
> 
> 	ret_val = qla4_83xx_set_win_base(ha, addr);
> 
>-	if (ret_val == QLA_SUCCESS)
>+	if (ret_val == QLA_SUCCESS) {
> 		*data = qla4_83xx_rd_reg(ha, QLA83XX_WILDCARD);
>-	else
>+	} else {
>+		*data = 0xffffffff;
> 		ql4_printk(KERN_ERR, ha, "%s: failed read of addr 0x%x!\n",
> 			   __func__, addr);
>+	}
> 
> 	return ret_val;
> }
>-- 
>2.7.0
>

Acked-by: Nilesh Javali <nilesh.javali@...gic.com>