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: <20170420175549.3435196-3-arnd@arndb.de>
Date:   Thu, 20 Apr 2017 19:54:47 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Johannes Berg <johannes.berg@...el.com>,
        Hannes Reinecke <hare@...e.com>, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH 3/4] scsi: pmcraid: fix endianess sparse annotations

The use of le32_to_cpu() etc in this driver looks completely arbitrary.
It may have made sense at some point, but it is not applied consistently,
so this driver presumably won't work on big-endian kernel builds.

Unfortunately it's unclear whether the type names or the calls to
le32_to_cpu() are the correct ones. I'm taking educated guesses here
and assume that most of the __le32 and __le16 annotations are correct,
adding the conversion helpers whereever we access those fields.

The exceptions are the 'fw_version' field that is always accessed as
big-endian, so I'm changing the type here, and the 'hrrq' values that
are accessed as little-endian, so I'm changing those the other way.

None of these changes should have any effect on little-endian
architectures like x86, but it addresses the sparse warnings.

Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 drivers/scsi/pmcraid.c | 95 +++++++++++++++++++++++++-------------------------
 drivers/scsi/pmcraid.h |  8 ++---
 2 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 35087e94c2ad..58c7511c5bd5 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -175,7 +175,7 @@ static int pmcraid_slave_alloc(struct scsi_device *scsi_dev)
 			if (fw_version <= PMCRAID_FW_VERSION_1)
 				target = temp->cfg_entry.unique_flags1;
 			else
-				target = temp->cfg_entry.array_id & 0xFF;
+				target = le16_to_cpu(temp->cfg_entry.array_id) & 0xFF;
 
 			if (target > PMCRAID_MAX_VSET_TARGETS)
 				continue;
@@ -330,7 +330,7 @@ static void pmcraid_init_cmdblk(struct pmcraid_cmd *cmd, int index)
 		ioarcb->request_flags0 = 0;
 		ioarcb->request_flags1 = 0;
 		ioarcb->cmd_timeout = 0;
-		ioarcb->ioarcb_bus_addr &= (~0x1FULL);
+		ioarcb->ioarcb_bus_addr &= cpu_to_le64(~0x1FULL);
 		ioarcb->ioadl_bus_addr = 0;
 		ioarcb->ioadl_length = 0;
 		ioarcb->data_transfer_length = 0;
@@ -898,8 +898,7 @@ static void _pmcraid_fire_command(struct pmcraid_cmd *cmd)
 
 	/* driver writes lower 32-bit value of IOARCB address only */
 	mb();
-	iowrite32(le32_to_cpu(cmd->ioa_cb->ioarcb.ioarcb_bus_addr),
-		  pinstance->ioarrin);
+	iowrite32(le64_to_cpu(cmd->ioa_cb->ioarcb.ioarcb_bus_addr), pinstance->ioarrin);
 }
 
 /**
@@ -1051,7 +1050,7 @@ static void pmcraid_get_fwversion(struct pmcraid_cmd *cmd)
 					offsetof(struct pmcraid_ioarcb,
 						add_data.u.ioadl[0]));
 	ioarcb->ioadl_length = cpu_to_le32(sizeof(struct pmcraid_ioadl_desc));
-	ioarcb->ioarcb_bus_addr &= ~(0x1FULL);
+	ioarcb->ioarcb_bus_addr &= cpu_to_le64(~(0x1FULL));
 
 	ioarcb->request_flags0 |= NO_LINK_DESCS;
 	ioarcb->data_transfer_length = cpu_to_le32(data_size);
@@ -1077,7 +1076,7 @@ static void pmcraid_identify_hrrq(struct pmcraid_cmd *cmd)
 	struct pmcraid_ioarcb *ioarcb = &cmd->ioa_cb->ioarcb;
 	int index = cmd->hrrq_index;
 	__be64 hrrq_addr = cpu_to_be64(pinstance->hrrq_start_bus_addr[index]);
-	u32 hrrq_size = cpu_to_be32(sizeof(u32) * PMCRAID_MAX_CMD);
+	__be32 hrrq_size = cpu_to_be32(sizeof(u32) * PMCRAID_MAX_CMD);
 	void (*done_function)(struct pmcraid_cmd *);
 
 	pmcraid_reinit_cmdblk(cmd);
@@ -1202,7 +1201,7 @@ static struct pmcraid_cmd *pmcraid_init_hcam
 
 	ioadl[0].flags |= IOADL_FLAGS_READ_LAST;
 	ioadl[0].data_len = cpu_to_le32(rcb_size);
-	ioadl[0].address = cpu_to_le32(dma);
+	ioadl[0].address = cpu_to_le64(dma);
 
 	cmd->cmd_done = cmd_done;
 	return cmd;
@@ -1237,7 +1236,13 @@ static void pmcraid_prepare_cancel_cmd(
 )
 {
 	struct pmcraid_ioarcb *ioarcb = &cmd->ioa_cb->ioarcb;
-	__be64 ioarcb_addr = cmd_to_cancel->ioa_cb->ioarcb.ioarcb_bus_addr;
+	__be64 ioarcb_addr;
+
+	/* IOARCB address of the command to be cancelled is given in
+	 * cdb[2]..cdb[9] is Big-Endian format. Note that length bits in
+	 * IOARCB address are not masked.
+	 */
+	ioarcb_addr = cpu_to_be64(le64_to_cpu(cmd_to_cancel->ioa_cb->ioarcb.ioarcb_bus_addr));
 
 	/* Get the resource handle to where the command to be aborted has been
 	 * sent.
@@ -1247,11 +1252,6 @@ static void pmcraid_prepare_cancel_cmd(
 	memset(ioarcb->cdb, 0, PMCRAID_MAX_CDB_LEN);
 	ioarcb->cdb[0] = PMCRAID_ABORT_CMD;
 
-	/* IOARCB address of the command to be cancelled is given in
-	 * cdb[2]..cdb[9] is Big-Endian format. Note that length bits in
-	 * IOARCB address are not masked.
-	 */
-	ioarcb_addr = cpu_to_be64(ioarcb_addr);
 	memcpy(&(ioarcb->cdb[2]), &ioarcb_addr, sizeof(ioarcb_addr));
 }
 
@@ -1493,7 +1493,7 @@ static int pmcraid_notify_ccn(struct pmcraid_instance *pinstance)
 {
 	return pmcraid_notify_aen(pinstance,
 				pinstance->ccn.msg,
-				pinstance->ccn.hcam->data_len +
+				le32_to_cpu(pinstance->ccn.hcam->data_len) +
 				sizeof(struct pmcraid_hcam_hdr));
 }
 
@@ -1508,7 +1508,7 @@ static int pmcraid_notify_ldn(struct pmcraid_instance *pinstance)
 {
 	return pmcraid_notify_aen(pinstance,
 				pinstance->ldn.msg,
-				pinstance->ldn.hcam->data_len +
+				le32_to_cpu(pinstance->ldn.hcam->data_len) +
 				sizeof(struct pmcraid_hcam_hdr));
 }
 
@@ -1556,10 +1556,10 @@ static void pmcraid_handle_config_change(struct pmcraid_instance *pinstance)
 
 	pmcraid_info("CCN(%x): %x timestamp: %llx type: %x lost: %x flags: %x \
 		 res: %x:%x:%x:%x\n",
-		 pinstance->ccn.hcam->ilid,
+		 le32_to_cpu(pinstance->ccn.hcam->ilid),
 		 pinstance->ccn.hcam->op_code,
-		((pinstance->ccn.hcam->timestamp1) |
-		((pinstance->ccn.hcam->timestamp2 & 0xffffffffLL) << 32)),
+		(le32_to_cpu(pinstance->ccn.hcam->timestamp1) |
+		((le32_to_cpu(pinstance->ccn.hcam->timestamp2) & 0xffffffffLL) << 32)),
 		 pinstance->ccn.hcam->notification_type,
 		 pinstance->ccn.hcam->notification_lost,
 		 pinstance->ccn.hcam->flags,
@@ -1570,7 +1570,7 @@ static void pmcraid_handle_config_change(struct pmcraid_instance *pinstance)
 		 RES_IS_VSET(*cfg_entry) ?
 			(fw_version <= PMCRAID_FW_VERSION_1 ?
 				cfg_entry->unique_flags1 :
-					cfg_entry->array_id & 0xFF) :
+				le16_to_cpu(cfg_entry->array_id) & 0xFF) :
 			RES_TARGET(cfg_entry->resource_address),
 		 RES_LUN(cfg_entry->resource_address));
 
@@ -1658,7 +1658,7 @@ static void pmcraid_handle_config_change(struct pmcraid_instance *pinstance)
 			if (fw_version <= PMCRAID_FW_VERSION_1)
 				res->cfg_entry.unique_flags1 &= 0x7F;
 			else
-				res->cfg_entry.array_id &= 0xFF;
+				res->cfg_entry.array_id &= cpu_to_le16(0xFF);
 			res->change_detected = RES_CHANGE_DEL;
 			res->cfg_entry.resource_handle =
 				PMCRAID_INVALID_RES_HANDLE;
@@ -1716,8 +1716,8 @@ static void pmcraid_ioasc_logger(u32 ioasc, struct pmcraid_cmd *cmd)
 	/* log the error string */
 	pmcraid_err("cmd [%x] for resource %x failed with %x(%s)\n",
 		cmd->ioa_cb->ioarcb.cdb[0],
-		cmd->ioa_cb->ioarcb.resource_handle,
-		le32_to_cpu(ioasc), error_info->error_string);
+		le32_to_cpu(cmd->ioa_cb->ioarcb.resource_handle),
+		ioasc, error_info->error_string);
 }
 
 /**
@@ -2034,7 +2034,7 @@ static void pmcraid_fail_outstanding_cmds(struct pmcraid_instance *pinstance)
 		cmd->ioa_cb->ioasa.ioasc =
 			cpu_to_le32(PMCRAID_IOASC_IOA_WAS_RESET);
 		cmd->ioa_cb->ioasa.ilid =
-			cpu_to_be32(PMCRAID_DRIVER_ILID);
+			cpu_to_le32(PMCRAID_DRIVER_ILID);
 
 		/* In case the command timer is still running */
 		del_timer(&cmd->timer);
@@ -2532,7 +2532,7 @@ static void pmcraid_cancel_all(struct pmcraid_cmd *cmd, u32 sense)
 	ioarcb->ioadl_bus_addr = 0;
 	ioarcb->ioadl_length = 0;
 	ioarcb->data_transfer_length = 0;
-	ioarcb->ioarcb_bus_addr &= (~0x1FULL);
+	ioarcb->ioarcb_bus_addr &= cpu_to_le64((~0x1FULL));
 
 	/* writing to IOARRIN must be protected by host_lock, as mid-layer
 	 * schedule queuecommand while we are doing this
@@ -2695,8 +2695,8 @@ static int pmcraid_error_handler(struct pmcraid_cmd *cmd)
 		 * mid-layer
 		 */
 		if (ioasa->auto_sense_length != 0) {
-			short sense_len = ioasa->auto_sense_length;
-			int data_size = min_t(u16, le16_to_cpu(sense_len),
+			short sense_len = le16_to_cpu(ioasa->auto_sense_length);
+			int data_size = min_t(u16, sense_len,
 					      SCSI_SENSE_BUFFERSIZE);
 
 			memcpy(scsi_cmd->sense_buffer,
@@ -2918,7 +2918,7 @@ static struct pmcraid_cmd *pmcraid_abort_cmd(struct pmcraid_cmd *cmd)
 
 	pmcraid_info("aborting command CDB[0]= %x with index = %d\n",
 		cmd->ioa_cb->ioarcb.cdb[0],
-		cmd->ioa_cb->ioarcb.response_handle >> 2);
+		le32_to_cpu(cmd->ioa_cb->ioarcb.response_handle) >> 2);
 
 	init_completion(&cancel_cmd->wait_for_completion);
 	cancel_cmd->completion_req = 1;
@@ -3143,9 +3143,8 @@ pmcraid_init_ioadls(struct pmcraid_cmd *cmd, int sgcount)
 	int ioadl_count = 0;
 
 	if (ioarcb->add_cmd_param_length)
-		ioadl_count = DIV_ROUND_UP(ioarcb->add_cmd_param_length, 16);
-	ioarcb->ioadl_length =
-		sizeof(struct pmcraid_ioadl_desc) * sgcount;
+		ioadl_count = DIV_ROUND_UP(le16_to_cpu(ioarcb->add_cmd_param_length), 16);
+	ioarcb->ioadl_length = cpu_to_le32(sizeof(struct pmcraid_ioadl_desc) * sgcount);
 
 	if ((sgcount + ioadl_count) > (ARRAY_SIZE(ioarcb->add_data.u.ioadl))) {
 		/* external ioadls start at offset 0x80 from control_block
@@ -3153,7 +3152,7 @@ pmcraid_init_ioadls(struct pmcraid_cmd *cmd, int sgcount)
 		 * It is necessary to indicate to firmware that driver is
 		 * using ioadls to be treated as external to IOARCB.
 		 */
-		ioarcb->ioarcb_bus_addr &= ~(0x1FULL);
+		ioarcb->ioarcb_bus_addr &= cpu_to_le64(~(0x1FULL));
 		ioarcb->ioadl_bus_addr =
 			cpu_to_le64((cmd->ioa_cb_bus_addr) +
 				offsetof(struct pmcraid_ioarcb,
@@ -3167,7 +3166,7 @@ pmcraid_init_ioadls(struct pmcraid_cmd *cmd, int sgcount)
 
 		ioadl = &ioarcb->add_data.u.ioadl[ioadl_count];
 		ioarcb->ioarcb_bus_addr |=
-				DIV_ROUND_CLOSEST(sgcount + ioadl_count, 8);
+			cpu_to_le64(DIV_ROUND_CLOSEST(sgcount + ioadl_count, 8));
 	}
 
 	return ioadl;
@@ -3493,7 +3492,7 @@ static int pmcraid_queuecommand_lck(
 		     RES_IS_VSET(res->cfg_entry) ?
 			(fw_version <= PMCRAID_FW_VERSION_1 ?
 				res->cfg_entry.unique_flags1 :
-					res->cfg_entry.array_id & 0xFF) :
+				le16_to_cpu(res->cfg_entry.array_id) & 0xFF) :
 			RES_TARGET(res->cfg_entry.resource_address),
 		     RES_LUN(res->cfg_entry.resource_address));
 
@@ -3709,7 +3708,7 @@ static long pmcraid_ioctl_passthrough(
 		goto out_free_buffer;
 	}
 
-	request_size = buffer->ioarcb.data_transfer_length;
+	request_size = le32_to_cpu(buffer->ioarcb.data_transfer_length);
 
 	if (buffer->ioarcb.request_flags0 & TRANSFER_DIR_WRITE) {
 		access = VERIFY_READ;
@@ -3732,7 +3731,8 @@ static long pmcraid_ioctl_passthrough(
 	}
 
 	/* check if we have any additional command parameters */
-	if (buffer->ioarcb.add_cmd_param_length > PMCRAID_ADD_CMD_PARAM_LEN) {
+	if (le16_to_cpu(buffer->ioarcb.add_cmd_param_length)
+	     > PMCRAID_ADD_CMD_PARAM_LEN) {
 		rc = -EINVAL;
 		goto out_free_buffer;
 	}
@@ -3764,7 +3764,7 @@ static long pmcraid_ioctl_passthrough(
 			buffer->ioarcb.add_cmd_param_offset;
 		memcpy(ioarcb->add_data.u.add_cmd_params,
 			buffer->ioarcb.add_data.u.add_cmd_params,
-			buffer->ioarcb.add_cmd_param_length);
+			le16_to_cpu(buffer->ioarcb.add_cmd_param_length));
 	}
 
 	/* set hrrq number where the IOA should respond to. Note that all cmds
@@ -3834,10 +3834,10 @@ static long pmcraid_ioctl_passthrough(
 		wait_for_completion(&cmd->wait_for_completion);
 	} else if (!wait_for_completion_timeout(
 			&cmd->wait_for_completion,
-			msecs_to_jiffies(buffer->ioarcb.cmd_timeout * 1000))) {
+			msecs_to_jiffies(le16_to_cpu(buffer->ioarcb.cmd_timeout) * 1000))) {
 
 		pmcraid_info("aborting cmd %d (CDB[0] = %x) due to timeout\n",
-			le32_to_cpu(cmd->ioa_cb->ioarcb.response_handle >> 2),
+			le32_to_cpu(cmd->ioa_cb->ioarcb.response_handle) >> 2,
 			cmd->ioa_cb->ioarcb.cdb[0]);
 
 		spin_lock_irqsave(pinstance->host->host_lock, lock_flags);
@@ -3846,7 +3846,7 @@ static long pmcraid_ioctl_passthrough(
 
 		if (cancel_cmd) {
 			wait_for_completion(&cancel_cmd->wait_for_completion);
-			ioasc = cancel_cmd->ioa_cb->ioasa.ioasc;
+			ioasc = le32_to_cpu(cancel_cmd->ioa_cb->ioasa.ioasc);
 			pmcraid_return_cmd(cancel_cmd);
 
 			/* if abort task couldn't find the command i.e it got
@@ -4461,7 +4461,7 @@ static void pmcraid_worker_function(struct work_struct *workp)
 				if (fw_version <= PMCRAID_FW_VERSION_1)
 					target = res->cfg_entry.unique_flags1;
 				else
-					target = res->cfg_entry.array_id & 0xFF;
+					target = le16_to_cpu(res->cfg_entry.array_id) & 0xFF;
 				lun = PMCRAID_VSET_LUN_ID;
 			} else {
 				bus = PMCRAID_PHYS_BUS_ID;
@@ -4500,7 +4500,7 @@ static void pmcraid_tasklet_function(unsigned long instance)
 	unsigned long host_lock_flags;
 	spinlock_t *lockp; /* hrrq buffer lock */
 	int id;
-	__le32 resp;
+	u32 resp;
 
 	hrrq_vector = (struct pmcraid_isr_param *)instance;
 	pinstance = hrrq_vector->drv_inst;
@@ -5540,8 +5540,7 @@ static void pmcraid_set_timestamp(struct pmcraid_cmd *cmd)
 	struct pmcraid_ioarcb *ioarcb = &cmd->ioa_cb->ioarcb;
 	__be32 time_stamp_len = cpu_to_be32(PMCRAID_TIMESTAMP_LEN);
 	struct pmcraid_ioadl_desc *ioadl = ioarcb->add_data.u.ioadl;
-
-	__le64 timestamp;
+	u64 timestamp;
 
 	timestamp = ktime_get_real_seconds() * 1000;
 
@@ -5563,7 +5562,7 @@ static void pmcraid_set_timestamp(struct pmcraid_cmd *cmd)
 					offsetof(struct pmcraid_ioarcb,
 						add_data.u.ioadl[0]));
 	ioarcb->ioadl_length = cpu_to_le32(sizeof(struct pmcraid_ioadl_desc));
-	ioarcb->ioarcb_bus_addr &= ~(0x1FULL);
+	ioarcb->ioarcb_bus_addr &= cpu_to_le64(~(0x1FULL));
 
 	ioarcb->request_flags0 |= NO_LINK_DESCS;
 	ioarcb->request_flags0 |= TRANSFER_DIR_WRITE;
@@ -5622,7 +5621,7 @@ static void pmcraid_init_res_table(struct pmcraid_cmd *cmd)
 	list_for_each_entry_safe(res, temp, &pinstance->used_res_q, queue)
 		list_move_tail(&res->queue, &old_res);
 
-	for (i = 0; i < pinstance->cfg_table->num_entries; i++) {
+	for (i = 0; i < le16_to_cpu(pinstance->cfg_table->num_entries); i++) {
 		if (be16_to_cpu(pinstance->inq_data->fw_version) <=
 						PMCRAID_FW_VERSION_1)
 			cfgte = &pinstance->cfg_table->entries[i];
@@ -5677,7 +5676,7 @@ static void pmcraid_init_res_table(struct pmcraid_cmd *cmd)
 				 res->cfg_entry.resource_type,
 				 (fw_version <= PMCRAID_FW_VERSION_1 ?
 					res->cfg_entry.unique_flags1 :
-						res->cfg_entry.array_id & 0xFF),
+					le16_to_cpu(res->cfg_entry.array_id) & 0xFF),
 				 le32_to_cpu(res->cfg_entry.resource_address));
 		}
 	}
@@ -5715,7 +5714,7 @@ static void pmcraid_querycfg(struct pmcraid_cmd *cmd)
 	struct pmcraid_ioarcb *ioarcb = &cmd->ioa_cb->ioarcb;
 	struct pmcraid_ioadl_desc *ioadl = ioarcb->add_data.u.ioadl;
 	struct pmcraid_instance *pinstance = cmd->drv_inst;
-	int cfg_table_size = cpu_to_be32(sizeof(struct pmcraid_config_table));
+	__be32 cfg_table_size = cpu_to_be32(sizeof(struct pmcraid_config_table));
 
 	if (be16_to_cpu(pinstance->inq_data->fw_version) <=
 					PMCRAID_FW_VERSION_1)
@@ -5740,7 +5739,7 @@ static void pmcraid_querycfg(struct pmcraid_cmd *cmd)
 					offsetof(struct pmcraid_ioarcb,
 						add_data.u.ioadl[0]));
 	ioarcb->ioadl_length = cpu_to_le32(sizeof(struct pmcraid_ioadl_desc));
-	ioarcb->ioarcb_bus_addr &= ~(0x1FULL);
+	ioarcb->ioarcb_bus_addr &= cpu_to_le64(~0x1FULL);
 
 	ioarcb->request_flags0 |= NO_LINK_DESCS;
 	ioarcb->data_transfer_length =
diff --git a/drivers/scsi/pmcraid.h b/drivers/scsi/pmcraid.h
index 568b18a2f47d..01eb2bc16dc1 100644
--- a/drivers/scsi/pmcraid.h
+++ b/drivers/scsi/pmcraid.h
@@ -554,7 +554,7 @@ struct pmcraid_inquiry_data {
 	__u8	add_page_len;
 	__u8	length;
 	__u8	reserved2;
-	__le16	fw_version;
+	__be16	fw_version;
 	__u8	reserved3[16];
 };
 
@@ -697,13 +697,13 @@ struct pmcraid_instance {
 	dma_addr_t hrrq_start_bus_addr[PMCRAID_NUM_MSIX_VECTORS];
 
 	/* Pointer to 1st entry of HRRQ */
-	__be32 *hrrq_start[PMCRAID_NUM_MSIX_VECTORS];
+	__le32 *hrrq_start[PMCRAID_NUM_MSIX_VECTORS];
 
 	/* Pointer to last entry of HRRQ */
-	__be32 *hrrq_end[PMCRAID_NUM_MSIX_VECTORS];
+	__le32 *hrrq_end[PMCRAID_NUM_MSIX_VECTORS];
 
 	/* Pointer to current pointer of hrrq */
-	__be32 *hrrq_curr[PMCRAID_NUM_MSIX_VECTORS];
+	__le32 *hrrq_curr[PMCRAID_NUM_MSIX_VECTORS];
 
 	/* Lock for HRRQ access */
 	spinlock_t hrrq_lock[PMCRAID_NUM_MSIX_VECTORS];
-- 
2.9.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ