* [PATCH] scsi: qla4xxx: Don't call dma_free_coherent without dma_alloc_coherent
@ 2019-03-07 18:41 Nathan Chancellor
2019-03-07 18:49 ` Nick Desaulniers
2019-03-07 23:15 ` [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL Nathan Chancellor
0 siblings, 2 replies; 5+ messages in thread
From: Nathan Chancellor @ 2019-03-07 18:41 UTC (permalink / raw)
To: QLogic-Storage-Upstream, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel, clang-built-linux, Nick Desaulniers,
Nathan Chancellor
When building with -Wsometimes-uninitialized, Clang warns:
drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is
used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
buf_dma is initialized by dma_alloc_coherent, which is only called in
the middle conditional statement. Use that same conditional to call
dma_free_coherent so that buf_dma is always initialized when used.
Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi")
Link: https://github.com/ClangBuiltLinux/linux/issues/391
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/scsi/qla4xxx/ql4_os.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 16a18d5d856f..88bb52456efd 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[])
ddb_index[1]));
exit_boot_info_free:
- dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
+ if (is_qla80XX(ha))
+ dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
exit_boot_info:
ha->pri_ddb_idx = ddb_index[0];
ha->sec_ddb_idx = ddb_index[1];
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qla4xxx: Don't call dma_free_coherent without dma_alloc_coherent
2019-03-07 18:41 [PATCH] scsi: qla4xxx: Don't call dma_free_coherent without dma_alloc_coherent Nathan Chancellor
@ 2019-03-07 18:49 ` Nick Desaulniers
2019-03-07 23:15 ` [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL Nathan Chancellor
1 sibling, 0 replies; 5+ messages in thread
From: Nick Desaulniers @ 2019-03-07 18:49 UTC (permalink / raw)
To: Nathan Chancellor
Cc: QLogic-Storage-Upstream, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML, clang-built-linux
On Thu, Mar 7, 2019 at 10:41 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is
> used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>
> buf_dma is initialized by dma_alloc_coherent, which is only called in
> the middle conditional statement. Use that same conditional to call
> dma_free_coherent so that buf_dma is always initialized when used.
>
> Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi")
> Link: https://github.com/ClangBuiltLinux/linux/issues/391
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/scsi/qla4xxx/ql4_os.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 16a18d5d856f..88bb52456efd 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[])
> ddb_index[1]));
>
> exit_boot_info_free:
> - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
> + if (is_qla80XX(ha))
> + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
Very tricky code flow to follow in this function, but this patch makes
sense upon closer inspection. Thanks for the patch Nathan
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL
2019-03-07 18:41 [PATCH] scsi: qla4xxx: Don't call dma_free_coherent without dma_alloc_coherent Nathan Chancellor
2019-03-07 18:49 ` Nick Desaulniers
@ 2019-03-07 23:15 ` Nathan Chancellor
2019-03-08 21:16 ` Nick Desaulniers
1 sibling, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2019-03-07 23:15 UTC (permalink / raw)
To: QLogic-Storage-Upstream, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel, clang-built-linux, Nick Desaulniers,
Nathan Chancellor
When building with -Wsometimes-uninitialized, Clang warns:
drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is
used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
Don't call dma_free_coherent when buf is NULL, meaning that we never
called dma_alloc_coherent and initialized buf_dma.
Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi")
Link: https://github.com/ClangBuiltLinux/linux/issues/391
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Check that buf is not NULL, which matches how dma_free_coherent is
called in the rest of the tree. This still fixes the warning.
drivers/scsi/qla4xxx/ql4_os.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 16a18d5d856f..9f56dafc3cda 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[])
ddb_index[1]));
exit_boot_info_free:
- dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
+ if (buf)
+ dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
exit_boot_info:
ha->pri_ddb_idx = ddb_index[0];
ha->sec_ddb_idx = ddb_index[1];
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL
2019-03-07 23:15 ` [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL Nathan Chancellor
@ 2019-03-08 21:16 ` Nick Desaulniers
2019-03-22 14:25 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Nick Desaulniers @ 2019-03-08 21:16 UTC (permalink / raw)
To: Nathan Chancellor
Cc: QLogic-Storage-Upstream, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML, clang-built-linux
On Thu, Mar 7, 2019 at 3:17 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is
> used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>
> Don't call dma_free_coherent when buf is NULL, meaning that we never
> called dma_alloc_coherent and initialized buf_dma.
>
> Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi")
> Link: https://github.com/ClangBuiltLinux/linux/issues/391
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2:
>
> * Check that buf is not NULL, which matches how dma_free_coherent is
> called in the rest of the tree. This still fixes the warning.
>
> drivers/scsi/qla4xxx/ql4_os.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 16a18d5d856f..9f56dafc3cda 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[])
> ddb_index[1]));
>
> exit_boot_info_free:
> - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
> + if (buf)
> + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
Same thoughts as: https://lkml.org/lkml/2019/3/8/786
WDYT?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL
2019-03-08 21:16 ` Nick Desaulniers
@ 2019-03-22 14:25 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, QLogic-Storage-Upstream, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, LKML, clang-built-linux
On Fri, Mar 8, 2019 at 10:17 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Mar 7, 2019 at 3:17 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is
> > used uninitialized whenever 'if' condition is false
> > [-Wsometimes-uninitialized]
> >
> > Don't call dma_free_coherent when buf is NULL, meaning that we never
> > called dma_alloc_coherent and initialized buf_dma.
> >
> > Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/391
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> > index 16a18d5d856f..9f56dafc3cda 100644
> > --- a/drivers/scsi/qla4xxx/ql4_os.c
> > +++ b/drivers/scsi/qla4xxx/ql4_os.c
> > @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[])
> > ddb_index[1]));
> >
> > exit_boot_info_free:
> > - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
> > + if (buf)
> > + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma);
>
> Same thoughts as: https://lkml.org/lkml/2019/3/8/786
> WDYT?
Initializing the variable is definitely worse here, don't listen to
clang's bad suggestions ;-)
This was my fix, I'll send that as a proper patch as well:
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -5928,7 +5928,7 @@ static int get_fw_boot_info(struct scsi_qla_host
*ha, uint16_t ddb_index[])
val = rd_nvram_byte(ha, sec_addr);
if (val & BIT_7)
ddb_index[1] = (val & 0x7f);
-
+ goto exit_boot_info;
} else if (is_qla80XX(ha)) {
buf = dma_alloc_coherent(&ha->pdev->dev, size,
&buf_dma, GFP_KERNEL);
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-22 14:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 18:41 [PATCH] scsi: qla4xxx: Don't call dma_free_coherent without dma_alloc_coherent Nathan Chancellor
2019-03-07 18:49 ` Nick Desaulniers
2019-03-07 23:15 ` [PATCH v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL Nathan Chancellor
2019-03-08 21:16 ` Nick Desaulniers
2019-03-22 14:25 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).