All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Arnd Bergmann <arnd@arndb.de>,
	James Bottomley <JBottomley@odin.com>,
	linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Matthew Wilcox <willy@linux.intel.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH, resend] scsi: advansys: fix big-endian builds
Date: Tue, 17 Nov 2015 07:46:01 +0100	[thread overview]
Message-ID: <564ACD29.3090007@suse.de> (raw)
In-Reply-To: <4411376.blKgWIg75B@wuerfel>

On 11/16/2015 05:49 PM, Arnd Bergmann wrote:
> Building the advansys driver in a big-endian configuration such as
> ARM allmodconfig shows a warning:
> 
>  drivers/scsi/advansys.c: In function 'adv_build_req':
>  include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>   #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
>  drivers/scsi/advansys.c:7806:22: note: in expansion of macro 'cpu_to_le32'
>    scsiqp->sense_len = cpu_to_le32(SCSI_SENSE_BUFFERSIZE);
> 
> It turns out that the commit that introduced this used the cpu_to_le32()
> incorrectly on an 8-bit field, which results in the sense_len to always
> be set to zero, as the SCSI_SENSE_BUFFERSIZE value gets moved to upper
> byte of the 32-bit intermediate.
> 
> This removes the cpu_to_le32() call to restore the original version.
> 
> I found this only by looking at the compiler output and have not done
> a full review for possible further endianess bugs in the same driver.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 811ddc057aac ("advansys: use DMA-API for mapping sense buffer")
> Cc: stable@vger.kernel.org # v4.2+
> ---
> Using willy@linux.intel.com, as the address listed in MAINTAINERS
> failed:
> 
> Failed to transport message. Message sending failed since the following recipients were rejected by the server: matthew@wil.cx (The server responded: Requested action not taken: mailbox unavailable invalid DNS MX or A/AAAA resource record)
> 
> Geert found the same bug and submitted the same patch earlier:
> https://lkml.org/lkml/2015/6/24/89
> 
> Neither one has been reviewed or accepted so far. Can we get one of the
> two merged please?
> 
> diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> index 4305178e4e01..1c1cd657c380 100644
> --- a/drivers/scsi/advansys.c
> +++ b/drivers/scsi/advansys.c
> @@ -7803,7 +7803,7 @@ adv_build_req(struct asc_board *boardp, struct scsi_cmnd *scp,
>  		return ASC_BUSY;
>  	}
>  	scsiqp->sense_addr = cpu_to_le32(sense_addr);
> -	scsiqp->sense_len = cpu_to_le32(SCSI_SENSE_BUFFERSIZE);
> +	scsiqp->sense_len = SCSI_SENSE_BUFFERSIZE;
>  
>  	/* Build ADV_SCSI_REQ_Q */
>  
> 
Ho-hum. You are right.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

WARNING: multiple messages have this Message-ID (diff)
From: hare@suse.de (Hannes Reinecke)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH, resend] scsi: advansys: fix big-endian builds
Date: Tue, 17 Nov 2015 07:46:01 +0100	[thread overview]
Message-ID: <564ACD29.3090007@suse.de> (raw)
In-Reply-To: <4411376.blKgWIg75B@wuerfel>

On 11/16/2015 05:49 PM, Arnd Bergmann wrote:
> Building the advansys driver in a big-endian configuration such as
> ARM allmodconfig shows a warning:
> 
>  drivers/scsi/advansys.c: In function 'adv_build_req':
>  include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>   #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
>  drivers/scsi/advansys.c:7806:22: note: in expansion of macro 'cpu_to_le32'
>    scsiqp->sense_len = cpu_to_le32(SCSI_SENSE_BUFFERSIZE);
> 
> It turns out that the commit that introduced this used the cpu_to_le32()
> incorrectly on an 8-bit field, which results in the sense_len to always
> be set to zero, as the SCSI_SENSE_BUFFERSIZE value gets moved to upper
> byte of the 32-bit intermediate.
> 
> This removes the cpu_to_le32() call to restore the original version.
> 
> I found this only by looking at the compiler output and have not done
> a full review for possible further endianess bugs in the same driver.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 811ddc057aac ("advansys: use DMA-API for mapping sense buffer")
> Cc: stable at vger.kernel.org # v4.2+
> ---
> Using willy at linux.intel.com, as the address listed in MAINTAINERS
> failed:
> 
> Failed to transport message. Message sending failed since the following recipients were rejected by the server: matthew at wil.cx (The server responded: Requested action not taken: mailbox unavailable invalid DNS MX or A/AAAA resource record)
> 
> Geert found the same bug and submitted the same patch earlier:
> https://lkml.org/lkml/2015/6/24/89
> 
> Neither one has been reviewed or accepted so far. Can we get one of the
> two merged please?
> 
> diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> index 4305178e4e01..1c1cd657c380 100644
> --- a/drivers/scsi/advansys.c
> +++ b/drivers/scsi/advansys.c
> @@ -7803,7 +7803,7 @@ adv_build_req(struct asc_board *boardp, struct scsi_cmnd *scp,
>  		return ASC_BUSY;
>  	}
>  	scsiqp->sense_addr = cpu_to_le32(sense_addr);
> -	scsiqp->sense_len = cpu_to_le32(SCSI_SENSE_BUFFERSIZE);
> +	scsiqp->sense_len = SCSI_SENSE_BUFFERSIZE;
>  
>  	/* Build ADV_SCSI_REQ_Q */
>  
> 
Ho-hum. You are right.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

  parent reply	other threads:[~2015-11-17  6:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-16 16:49 [PATCH, resend] scsi: advansys: fix big-endian builds Arnd Bergmann
2015-11-16 16:49 ` Arnd Bergmann
2015-11-16 18:20 ` Russell King - ARM Linux
2015-11-16 18:20   ` Russell King - ARM Linux
2015-11-16 18:35   ` Christoph Hellwig
2015-11-16 18:35     ` Christoph Hellwig
2015-11-16 18:56   ` Geert Uytterhoeven
2015-11-16 18:56     ` Geert Uytterhoeven
2015-11-17  6:46 ` Hannes Reinecke [this message]
2015-11-17  6:46   ` Hannes Reinecke
2015-11-18 15:18 ` Martin K. Petersen
2015-11-18 15:18   ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=564ACD29.3090007@suse.de \
    --to=hare@suse.de \
    --cc=JBottomley@odin.com \
    --cc=arnd@arndb.de \
    --cc=geert@linux-m68k.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.