[PATCH v2 0/2] Fixes for issues detected by static analyzers

[PATCH v2 0/2] Fixes for issues detected by static analyzers

[Bart Van Assche]

Hi Keith, Christoph and Sagi,

This patch series addresses several issues reported by static analyzers like
sparse, smatch and Coverity. Please consider these patches for Linux kernel
v4.20.

Thanks,

Bart.

Changes compared to v1:
- Left out the patches that have already been queued by Christoph and also
  patch "nvme-core: Complain if nvme_init_identify() fails".
- Addressed the review feedback that I received for the two patches in v2
  of this series.

Bart Van Assche (2):
  nvme-core: Make implicit seed truncation explicit
  nvmet-fcloop: Suppress a compiler warning

 drivers/nvme/host/core.c     | 2 +-
 drivers/nvme/target/fcloop.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.19.0.605.g01d371f741-goog

[PATCH v2 1/2] nvme-core: Make implicit seed truncation explicit

[Bart Van Assche]

The nvme_user_io.slba field is 64 bits wide. That value is copied into the
32-bit bio_integrity_payload.bip_iter.bi_sector field. Make that truncation
explicit to avoid that Coverity complains about implicit truncation. See
also Coverity ID 1056486 on http://scan.coverity.com/projects/linux.

Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8cecb36b5af1..65c42448e904 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1132,7 +1132,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 
 	return nvme_submit_user_cmd(ns->queue, &c,
 			(void __user *)(uintptr_t)io.addr, length,
-			metadata, meta_len, io.slba, NULL, 0);
+			metadata, meta_len, lower_32_bits(io.slba), NULL, 0);
 }
 
 static u32 nvme_known_admin_effects(u8 opcode)
-- 
2.19.0.605.g01d371f741-goog

[PATCH v2 2/2] nvmet-fcloop: Suppress a compiler warning

[Bart Van Assche]

Building with W=1 enables the compiler warning -Wimplicit-fallthrough=3. That
option does not recognize the fall-through comment in the fcloop driver. Add
a fall-through comment that is recognized for -Wimplicit-fallthrough=3. This
patch avoids that the compiler reports the following warning when building
with W=1:

drivers/nvme/target/fcloop.c:647:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
   if (op == NVMET_FCOP_READDATA)
      ^

Cc: James Smart <james.smart at broadcom.com>
Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
 drivers/nvme/target/fcloop.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index 5251689a1d9a..291f4121f516 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -648,6 +648,7 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport,
 			break;
 
 		/* Fall-Thru to RSP handling */
+		/* FALLTHRU */
 
 	case NVMET_FCOP_RSP:
 		if (fcpreq) {
-- 
2.19.0.605.g01d371f741-goog

[PATCH v2 1/2] nvme-core: Make implicit seed truncation explicit

[Keith Busch]

On Wed, Oct 10, 2018@08:08:19AM -0700, Bart Van Assche wrote:
> The nvme_user_io.slba field is 64 bits wide. That value is copied into the
> 32-bit bio_integrity_payload.bip_iter.bi_sector field. Make that truncation
> explicit to avoid that Coverity complains about implicit truncation. See
> also Coverity ID 1056486 on http://scan.coverity.com/projects/linux.
> 
> Signed-off-by: Bart Van Assche <bvanassche at acm.org>

Looks good, thanks for the update.

Reviewed-by: Keith Busch <keith.busch at intel.com>


>  drivers/nvme/host/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 8cecb36b5af1..65c42448e904 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1132,7 +1132,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
>  
>  	return nvme_submit_user_cmd(ns->queue, &c,
>  			(void __user *)(uintptr_t)io.addr, length,
> -			metadata, meta_len, io.slba, NULL, 0);
> +			metadata, meta_len, lower_32_bits(io.slba), NULL, 0);
>  }
>  
>  static u32 nvme_known_admin_effects(u8 opcode)
> -- 
> 2.19.0.605.g01d371f741-goog
> 

[PATCH v2 2/2] nvmet-fcloop: Suppress a compiler warning

[James Smart]

On 10/10/2018 8:08 AM, Bart Van Assche wrote:
> Building with W=1 enables the compiler warning -Wimplicit-fallthrough=3. That
> option does not recognize the fall-through comment in the fcloop driver. Add
> a fall-through comment that is recognized for -Wimplicit-fallthrough=3. This
> patch avoids that the compiler reports the following warning when building
> with W=1:
>
> drivers/nvme/target/fcloop.c:647:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     if (op == NVMET_FCOP_READDATA)
>        ^
>
> Cc: James Smart <james.smart at broadcom.com>
> Signed-off-by: Bart Van Assche <bvanassche at acm.org>
> ---
>   drivers/nvme/target/fcloop.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index 5251689a1d9a..291f4121f516 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -648,6 +648,7 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport,
>   			break;
>   
>   		/* Fall-Thru to RSP handling */
> +		/* FALLTHRU */
>   
>   	case NVMET_FCOP_RSP:
>   		if (fcpreq) {

Reviewed-by: James Smart <james.smart at broadcom.com>

I had no idea there was a "standard" way to declare this.

-- james

[PATCH v2 2/2] nvmet-fcloop: Suppress a compiler warning

[Bart Van Assche]

On Wed, 2018-10-10@08:57 -0700, James Smart wrote:
> On 10/10/2018 8:08 AM, Bart Van Assche wrote:
> > diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> > index 5251689a1d9a..291f4121f516 100644
> > --- a/drivers/nvme/target/fcloop.c
> > +++ b/drivers/nvme/target/fcloop.c
> > @@ -648,6 +648,7 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport,
> >   			break;
> >   
> >   		/* Fall-Thru to RSP handling */
> > +		/* FALLTHRU */
> >   
> >   	case NVMET_FCOP_RSP:
> >   		if (fcpreq) {
> 
> Reviewed-by: James Smart <james.smart at broadcom.com>
> 
> I had no idea there was a "standard" way to declare this.

Thanks James for the review. A detailed description of which comments are recognized by
gcc is available on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options:

The option argument n specifies what kind of comments are accepted:

-Wimplicit-fallthrough=0 disables the warning altogether.
-Wimplicit-fallthrough=1 matches .* regular expression, any comment is used as fallthrough comment.
-Wimplicit-fallthrough=2 case insensitively matches .*falls?[ \t-]*thr(ough|u).* regular expression.
-Wimplicit-fallthrough=3 case sensitively matches one of the following regular expressions:
-fallthrough
@fallthrough@
lint -fallthrough[ \t]*
[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?
FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*(Else,? |Intentional(ly)? )?
Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?
-Wimplicit-fallthrough=4 case sensitively matches one of the following regular expressions:
-fallthrough
@fallthrough@
lint -fallthrough[ \t]*
[ \t]*FALLTHR(OUGH|U)[ \t]*
-Wimplicit-fallthrough=5 doesn?t recognize any comments as fallthrough comments, only attributes disable the warning.

Bart.

[PATCH v2 0/2] Fixes for issues detected by static analyzers

[Christoph Hellwig]

Thanks a lot Bart,

applied to nvme-4.20.