2018-10-19 17:59:26

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers,
which don't find into type 'int', which is used for the cmd paremeter in
the ioctls in scsi_host_template. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful. However, looking at
the rest of the kernel tree, all ioctls use an 'unsigned int' for the
cmd parameter, which will fit all of the _IOC values in the scsi/ata
subsystems.

Make that change because none of the ioctls expect to take a negative
value, it brings the ioctls inline with the reset of the kernel, and it
removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/ata/libata-scsi.c | 5 +++--
drivers/scsi/aacraid/aachba.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 4 ++--
drivers/scsi/aacraid/commctrl.c | 2 +-
drivers/scsi/aacraid/linit.c | 6 ++++--
drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
drivers/scsi/esas2r/esas2r.h | 4 ++--
drivers/scsi/esas2r/esas2r_ioctl.c | 13 ++++++-------
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/hpsa.c | 15 +++++++++------
drivers/scsi/ipr.c | 3 ++-
drivers/scsi/libsas/sas_scsi_host.c | 2 +-
drivers/scsi/scsi_debug.c | 3 ++-
drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
include/linux/libata.h | 5 +++--
include/scsi/libsas.h | 3 ++-
include/scsi/scsi_host.h | 6 ++++--
17 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3d4887d0e84a..6291f1dbf342 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
}

int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
unsigned long val;
int rc = -EINVAL;
@@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
}
EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);

-int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
+int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
+ void __user *arg)
{
return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
scsidev, cmd, arg);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index bd7f352c28f3..a6399fac3333 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3452,7 +3452,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
}
}

-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
switch (cmd) {
case FSACTL_QUERY_DISK:
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 39eb415987fc..1965072ab673 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2705,12 +2705,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
int aac_get_config_status(struct aac_dev *dev, int commit_flag);
int aac_get_containers(struct aac_dev *dev);
int aac_scsi_cmd(struct scsi_cmnd *cmd);
-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
#ifndef shost_to_class
#define shost_to_class(shost) &shost->shost_dev
#endif
ssize_t aac_get_serial_number(struct device *dev, char *buf);
-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
int aac_rx_init(struct aac_dev *dev);
int aac_rkt_init(struct aac_dev *dev);
int aac_nark_init(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 25f6600d6c09..fd8a10871598 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1061,7 +1061,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
return retval;
}

-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
int status;

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 2d4e4ddc5ace..cd2efb44f62c 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
NULL,
};

-static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
+static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
@@ -1205,7 +1206,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
return ret;
}

-static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index acac6152f50b..1a94a469051e 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
*
* Return: A string identifying the decoded ioctl.
*/
-static char *decode_ioctl(int cmd)
+static char *decode_ioctl(unsigned int cmd)
{
switch (cmd) {
case DK_CXLFLASH_ATTACH:
@@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
*
* Return: 0 on success, -errno on failure
*/
-static int ioctl_common(struct scsi_device *sdev, int cmd)
+static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
{
struct cxlflash_cfg *cfg = shost_priv(sdev->host);
struct device *dev = &cfg->dev->dev;
@@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
*
* Return: 0 on success, -errno on failure
*/
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
typedef int (*sioctl) (struct scsi_device *, void *);

@@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
}

if (unlikely(copy_from_user(&buf, arg, size))) {
- dev_err(dev, "%s: copy_from_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
goto cxlflash_ioctl_exit;
@@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
rc = do_ioctl(sdev, (void *)&buf);
if (likely(!rc))
if (unlikely(copy_to_user(arg, &buf, size))) {
- dev_err(dev, "%s: copy_to_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
}
diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 858c3b33db78..7f43b95f4e94 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -965,8 +965,8 @@ struct esas2r_adapter {
const char *esas2r_info(struct Scsi_Host *);
int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
-int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
+int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
u8 handle_hba_ioctl(struct esas2r_adapter *a,
struct atto_ioctl *ioctl_hba);
int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
index 34bcc8c04ff4..916b9adf4921 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,


/* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
{
struct atto_express_ioctl *ioctl = NULL;
struct esas2r_adapter *a;
@@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
if (IS_ERR(ioctl)) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler access_ok failed for cmd %d, "
+ "ioctl_handler access_ok failed for cmd %u, "
"address %p", cmd,
arg);
return PTR_ERR(ioctl);
@@ -1493,7 +1493,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl_done:

if (err < 0) {
- esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
+ esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
cmd);

switch (err) {
@@ -1518,9 +1518,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
if (err != 0) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler copy_to_user didn't copy "
- "everything (err %d, cmd %d)", err,
- cmd);
+ "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
+ err, cmd);
kfree(ioctl);

return -EFAULT;
@@ -1531,7 +1530,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
return 0;
}

-int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
+int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
{
return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
}
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index c07118617d89..25dc8d44ed8e 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -624,7 +624,7 @@ static int esas2r_proc_major;
long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
- (int)cmd, (void __user *)arg);
+ cmd, (void __user *)arg);
}

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c9cccf35e9d7..c9d41d3b50f1 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);

#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
void __user *arg);
#endif

@@ -6123,7 +6124,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)

#ifdef CONFIG_COMPAT

-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
@@ -6160,7 +6161,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
}

static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -6197,7 +6198,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}

-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -6517,7 +6519,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 271990bc065b..97fcc881f9f1 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
* Return value:
* 0 on success / other on failure
**/
-static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct ipr_resource_entry *res;

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 33229348dcb6..4eeaafbe0504 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -805,7 +805,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
shost->host_failed, tries);
}

-int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 60bcc6df97a9..4c513651c95f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
}

-static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
if (sdebug_verbose) {
if (0x1261 == cmd)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index a25a07a0b7f0..498bdfbbebfa 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -5572,7 +5572,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

-static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 38c95d66ab12..263ca4f05c24 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1123,10 +1123,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
extern void ata_host_detach(struct ata_host *host);
extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
extern int ata_scsi_detect(struct scsi_host_template *sht);
-extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
- int cmd, void __user *arg);
+ unsigned int cmd, void __user *arg);
extern void ata_sas_port_destroy(struct ata_port *);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 3de3b10da19a..70461264d6e7 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -709,7 +709,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);

extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
-extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
extern int sas_drain_work(struct sas_ha_struct *ha);

extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5ea06d310a25..5c1df26e1d4f 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -65,7 +65,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);


#ifdef CONFIG_COMPAT
@@ -75,7 +76,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
#endif

/*
--
2.19.1



2018-10-19 19:39:31

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Fri, 2018-10-19 at 10:57 -0700, Nathan Chancellor wrote:
+AD4 Clang warns several times in the scsi subsystem (trimmed for brevity):
+AD4
+AD4 drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
+AD4 switch condition type (2147762695 to 18446744071562347015) +AFs--Wswitch+AF0
+AD4 case CCISS+AF8-GETBUSTYPES:
+AD4 +AF4
+AD4 drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
+AD4 switch condition type (2147762694 to 18446744071562347014) +AFs--Wswitch+AF0
+AD4 case CCISS+AF8-GETHEARTBEAT:
+AD4 +AF4
+AD4
+AD4 The root cause is that the +AF8-IOC macro can generate really large numbers,
+AD4 which don't find into type 'int', which is used for the cmd paremeter in
+AD4 the ioctls in scsi+AF8-host+AF8-template. My research into how GCC and Clang are
+AD4 handling this at a low level didn't prove fruitful. However, looking at
+AD4 the rest of the kernel tree, all ioctls use an 'unsigned int' for the
+AD4 cmd parameter, which will fit all of the +AF8-IOC values in the scsi/ata
+AD4 subsystems.
+AD4
+AD4 Make that change because none of the ioctls expect to take a negative
+AD4 value, it brings the ioctls inline with the reset of the kernel, and it
+AD4 removes ambiguity, which is never good when dealing with compilers.

Reviewed-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4


2018-10-20 04:54:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Hi Nathan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.19-rc8 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Nathan-Chancellor/scsi-ata-Use-unsigned-int-for-cmd-s-type-in-ioctls-in-scsi_host_template/20181020-120416
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc

All errors (new ones prefixed by >>):

>> drivers/scsi//cxlflash/main.c:3170:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.ioctl = cxlflash_ioctl,
^~~~~~~~~~~~~~
drivers/scsi//cxlflash/main.c:3170:11: note: (near initialization for 'driver_template.ioctl')
cc1: some warnings being treated as errors
--
>> drivers/scsi//cxlflash/superpipe.c:2099:5: error: conflicting types for 'cxlflash_ioctl'
int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
^~~~~~~~~~~~~~
In file included from drivers/scsi//cxlflash/superpipe.c:29:0:
drivers/scsi//cxlflash/common.h:337:5: note: previous declaration of 'cxlflash_ioctl' was here
int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
^~~~~~~~~~~~~~

vim +/cxlflash_ioctl +2099 drivers/scsi//cxlflash/superpipe.c

2082
2083 /**
2084 * cxlflash_ioctl() - IOCTL handler for driver
2085 * @sdev: SCSI device associated with LUN.
2086 * @cmd: IOCTL command.
2087 * @arg: Userspace ioctl data structure.
2088 *
2089 * A read/write semaphore is used to implement a 'drain' of currently
2090 * running ioctls. The read semaphore is taken at the beginning of each
2091 * ioctl thread and released upon concluding execution. Additionally the
2092 * semaphore should be released and then reacquired in any ioctl execution
2093 * path which will wait for an event to occur that is outside the scope of
2094 * the ioctl (i.e. an adapter reset). To drain the ioctls currently running,
2095 * a thread simply needs to acquire the write semaphore.
2096 *
2097 * Return: 0 on success, -errno on failure
2098 */
> 2099 int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.83 kB)
.config.gz (57.22 kB)
Download all attachments

2018-10-20 05:03:38

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers,
which don't find into type 'int', which is used for the cmd paremeter in
the ioctls in scsi_host_template. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful. However, looking at
the rest of the kernel tree, all ioctls use an 'unsigned int' for the
cmd parameter, which will fit all of the _IOC values in the scsi/ata
subsystems.

Make that change because none of the ioctls expect to take a negative
value, it brings the ioctls inline with the reset of the kernel, and it
removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
---

v1 -> 2:

* Fix build breakage in cxlflash driver from forgetting to update
prototype (thanks to 0day for catching it).

drivers/ata/libata-scsi.c | 5 +++--
drivers/scsi/aacraid/aachba.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 4 ++--
drivers/scsi/aacraid/commctrl.c | 2 +-
drivers/scsi/aacraid/linit.c | 6 ++++--
drivers/scsi/cxlflash/common.h | 3 ++-
drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
drivers/scsi/esas2r/esas2r.h | 4 ++--
drivers/scsi/esas2r/esas2r_ioctl.c | 13 ++++++-------
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/hpsa.c | 15 +++++++++------
drivers/scsi/ipr.c | 3 ++-
drivers/scsi/libsas/sas_scsi_host.c | 2 +-
drivers/scsi/scsi_debug.c | 3 ++-
drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
include/linux/libata.h | 5 +++--
include/scsi/libsas.h | 3 ++-
include/scsi/scsi_host.h | 6 ++++--
18 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3d4887d0e84a..6291f1dbf342 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
}

int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
unsigned long val;
int rc = -EINVAL;
@@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
}
EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);

-int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
+int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
+ void __user *arg)
{
return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
scsidev, cmd, arg);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index bd7f352c28f3..a6399fac3333 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3452,7 +3452,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
}
}

-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
switch (cmd) {
case FSACTL_QUERY_DISK:
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 39eb415987fc..1965072ab673 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2705,12 +2705,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
int aac_get_config_status(struct aac_dev *dev, int commit_flag);
int aac_get_containers(struct aac_dev *dev);
int aac_scsi_cmd(struct scsi_cmnd *cmd);
-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
#ifndef shost_to_class
#define shost_to_class(shost) &shost->shost_dev
#endif
ssize_t aac_get_serial_number(struct device *dev, char *buf);
-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
int aac_rx_init(struct aac_dev *dev);
int aac_rkt_init(struct aac_dev *dev);
int aac_nark_init(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 25f6600d6c09..fd8a10871598 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1061,7 +1061,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
return retval;
}

-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
int status;

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 2d4e4ddc5ace..cd2efb44f62c 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
NULL,
};

-static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
+static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
@@ -1205,7 +1206,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
return ret;
}

-static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 8908a20065c8..4d90106fcb37 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
void cxlflash_free_errpage(void);
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index acac6152f50b..1a94a469051e 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
*
* Return: A string identifying the decoded ioctl.
*/
-static char *decode_ioctl(int cmd)
+static char *decode_ioctl(unsigned int cmd)
{
switch (cmd) {
case DK_CXLFLASH_ATTACH:
@@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
*
* Return: 0 on success, -errno on failure
*/
-static int ioctl_common(struct scsi_device *sdev, int cmd)
+static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
{
struct cxlflash_cfg *cfg = shost_priv(sdev->host);
struct device *dev = &cfg->dev->dev;
@@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
*
* Return: 0 on success, -errno on failure
*/
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
typedef int (*sioctl) (struct scsi_device *, void *);

@@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
}

if (unlikely(copy_from_user(&buf, arg, size))) {
- dev_err(dev, "%s: copy_from_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
goto cxlflash_ioctl_exit;
@@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
rc = do_ioctl(sdev, (void *)&buf);
if (likely(!rc))
if (unlikely(copy_to_user(arg, &buf, size))) {
- dev_err(dev, "%s: copy_to_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
}
diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 858c3b33db78..7f43b95f4e94 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -965,8 +965,8 @@ struct esas2r_adapter {
const char *esas2r_info(struct Scsi_Host *);
int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
-int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
+int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
u8 handle_hba_ioctl(struct esas2r_adapter *a,
struct atto_ioctl *ioctl_hba);
int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
index 34bcc8c04ff4..916b9adf4921 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,


/* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
{
struct atto_express_ioctl *ioctl = NULL;
struct esas2r_adapter *a;
@@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
if (IS_ERR(ioctl)) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler access_ok failed for cmd %d, "
+ "ioctl_handler access_ok failed for cmd %u, "
"address %p", cmd,
arg);
return PTR_ERR(ioctl);
@@ -1493,7 +1493,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl_done:

if (err < 0) {
- esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
+ esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
cmd);

switch (err) {
@@ -1518,9 +1518,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
if (err != 0) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler copy_to_user didn't copy "
- "everything (err %d, cmd %d)", err,
- cmd);
+ "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
+ err, cmd);
kfree(ioctl);

return -EFAULT;
@@ -1531,7 +1530,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
return 0;
}

-int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
+int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
{
return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
}
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index c07118617d89..25dc8d44ed8e 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -624,7 +624,7 @@ static int esas2r_proc_major;
long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
- (int)cmd, (void __user *)arg);
+ cmd, (void __user *)arg);
}

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c9cccf35e9d7..c9d41d3b50f1 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);

#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
void __user *arg);
#endif

@@ -6123,7 +6124,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)

#ifdef CONFIG_COMPAT

-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
@@ -6160,7 +6161,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
}

static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -6197,7 +6198,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}

-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -6517,7 +6519,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 271990bc065b..97fcc881f9f1 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
* Return value:
* 0 on success / other on failure
**/
-static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct ipr_resource_entry *res;

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 33229348dcb6..4eeaafbe0504 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -805,7 +805,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
shost->host_failed, tries);
}

-int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 60bcc6df97a9..4c513651c95f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
}

-static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
if (sdebug_verbose) {
if (0x1261 == cmd)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index a25a07a0b7f0..498bdfbbebfa 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -5572,7 +5572,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

-static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 38c95d66ab12..263ca4f05c24 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1123,10 +1123,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
extern void ata_host_detach(struct ata_host *host);
extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
extern int ata_scsi_detect(struct scsi_host_template *sht);
-extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
- int cmd, void __user *arg);
+ unsigned int cmd, void __user *arg);
extern void ata_sas_port_destroy(struct ata_port *);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 3de3b10da19a..70461264d6e7 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -709,7 +709,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);

extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
-extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
extern int sas_drain_work(struct sas_ha_struct *ha);

extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5ea06d310a25..5c1df26e1d4f 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -65,7 +65,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);


#ifdef CONFIG_COMPAT
@@ -75,7 +76,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
#endif

/*
--
2.19.1


2018-12-17 19:38:11

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Fri, Oct 19, 2018 at 10:01:44PM -0700, Nathan Chancellor wrote:
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't find into type 'int', which is used for the cmd paremeter in
> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect to take a negative
> value, it brings the ioctls inline with the reset of the kernel, and it
> removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
>
> v1 -> 2:
>
> * Fix build breakage in cxlflash driver from forgetting to update
> prototype (thanks to 0day for catching it).
>
> drivers/ata/libata-scsi.c | 5 +++--
> drivers/scsi/aacraid/aachba.c | 2 +-
> drivers/scsi/aacraid/aacraid.h | 4 ++--
> drivers/scsi/aacraid/commctrl.c | 2 +-
> drivers/scsi/aacraid/linit.c | 6 ++++--
> drivers/scsi/cxlflash/common.h | 3 ++-
> drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
> drivers/scsi/esas2r/esas2r.h | 4 ++--
> drivers/scsi/esas2r/esas2r_ioctl.c | 13 ++++++-------
> drivers/scsi/esas2r/esas2r_main.c | 2 +-
> drivers/scsi/hpsa.c | 15 +++++++++------
> drivers/scsi/ipr.c | 3 ++-
> drivers/scsi/libsas/sas_scsi_host.c | 2 +-
> drivers/scsi/scsi_debug.c | 3 ++-
> drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
> include/linux/libata.h | 5 +++--
> include/scsi/libsas.h | 3 ++-
> include/scsi/scsi_host.h | 6 ++++--
> 18 files changed, 52 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3d4887d0e84a..6291f1dbf342 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
> }
>
> int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> unsigned long val;
> int rc = -EINVAL;
> @@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> }
> EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);
>
> -int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
> +int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
> + void __user *arg)
> {
> return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
> scsidev, cmd, arg);
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index bd7f352c28f3..a6399fac3333 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -3452,7 +3452,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
> }
> }
>
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> switch (cmd) {
> case FSACTL_QUERY_DISK:
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 39eb415987fc..1965072ab673 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2705,12 +2705,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
> int aac_get_config_status(struct aac_dev *dev, int commit_flag);
> int aac_get_containers(struct aac_dev *dev);
> int aac_scsi_cmd(struct scsi_cmnd *cmd);
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> #ifndef shost_to_class
> #define shost_to_class(shost) &shost->shost_dev
> #endif
> ssize_t aac_get_serial_number(struct device *dev, char *buf);
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> int aac_rx_init(struct aac_dev *dev);
> int aac_rkt_init(struct aac_dev *dev);
> int aac_nark_init(struct aac_dev *dev);
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 25f6600d6c09..fd8a10871598 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -1061,7 +1061,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
> return retval;
> }
>
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> int status;
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 2d4e4ddc5ace..cd2efb44f62c 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
> NULL,
> };
>
> -static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> +static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> @@ -1205,7 +1206,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
> return ret;
> }
>
> -static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 8908a20065c8..4d90106fcb37 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
> void cxlflash_list_init(void);
> void cxlflash_term_global_luns(void);
> void cxlflash_free_errpage(void);
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
> int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
> void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
> index acac6152f50b..1a94a469051e 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
> *
> * Return: A string identifying the decoded ioctl.
> */
> -static char *decode_ioctl(int cmd)
> +static char *decode_ioctl(unsigned int cmd)
> {
> switch (cmd) {
> case DK_CXLFLASH_ATTACH:
> @@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
> *
> * Return: 0 on success, -errno on failure
> */
> -static int ioctl_common(struct scsi_device *sdev, int cmd)
> +static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
> {
> struct cxlflash_cfg *cfg = shost_priv(sdev->host);
> struct device *dev = &cfg->dev->dev;
> @@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
> *
> * Return: 0 on success, -errno on failure
> */
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> typedef int (*sioctl) (struct scsi_device *, void *);
>
> @@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> }
>
> if (unlikely(copy_from_user(&buf, arg, size))) {
> - dev_err(dev, "%s: copy_from_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> goto cxlflash_ioctl_exit;
> @@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> rc = do_ioctl(sdev, (void *)&buf);
> if (likely(!rc))
> if (unlikely(copy_to_user(arg, &buf, size))) {
> - dev_err(dev, "%s: copy_to_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> }
> diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
> index 858c3b33db78..7f43b95f4e94 100644
> --- a/drivers/scsi/esas2r/esas2r.h
> +++ b/drivers/scsi/esas2r/esas2r.h
> @@ -965,8 +965,8 @@ struct esas2r_adapter {
> const char *esas2r_info(struct Scsi_Host *);
> int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
> struct esas2r_sas_nvram *data);
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
> -int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
> +int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
> u8 handle_hba_ioctl(struct esas2r_adapter *a,
> struct atto_ioctl *ioctl_hba);
> int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 34bcc8c04ff4..916b9adf4921 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
>
>
> /* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
> {
> struct atto_express_ioctl *ioctl = NULL;
> struct esas2r_adapter *a;
> @@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> if (IS_ERR(ioctl)) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler access_ok failed for cmd %d, "
> + "ioctl_handler access_ok failed for cmd %u, "
> "address %p", cmd,
> arg);
> return PTR_ERR(ioctl);
> @@ -1493,7 +1493,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl_done:
>
> if (err < 0) {
> - esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
> + esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
> cmd);
>
> switch (err) {
> @@ -1518,9 +1518,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
> if (err != 0) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler copy_to_user didn't copy "
> - "everything (err %d, cmd %d)", err,
> - cmd);
> + "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
> + err, cmd);
> kfree(ioctl);
>
> return -EFAULT;
> @@ -1531,7 +1530,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> return 0;
> }
>
> -int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
> +int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
> {
> return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
> }
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index c07118617d89..25dc8d44ed8e 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -624,7 +624,7 @@ static int esas2r_proc_major;
> long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
> return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
> - (int)cmd, (void __user *)arg);
> + cmd, (void __user *)arg);
> }
>
> static void __exit esas2r_exit(void)
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index c9cccf35e9d7..c9d41d3b50f1 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -251,10 +251,11 @@ static int number_of_controllers;
>
> static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
> static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
> #ifdef CONFIG_COMPAT
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> void __user *arg);
> #endif
>
> @@ -6123,7 +6124,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)
>
> #ifdef CONFIG_COMPAT
>
> -static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> +static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
> void __user *arg)
> {
> IOCTL32_Command_struct __user *arg32 =
> @@ -6160,7 +6161,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> }
>
> static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> BIG_IOCTL32_Command_struct __user *arg32 =
> (BIG_IOCTL32_Command_struct __user *) arg;
> @@ -6197,7 +6198,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> return err;
> }
>
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> switch (cmd) {
> case CCISS_GETPCIINFO:
> @@ -6517,7 +6519,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
> /*
> * ioctl
> */
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> struct ctlr_info *h;
> void __user *argp = (void __user *)arg;
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index 271990bc065b..97fcc881f9f1 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
> * Return value:
> * 0 on success / other on failure
> **/
> -static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct ipr_resource_entry *res;
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index 33229348dcb6..4eeaafbe0504 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -805,7 +805,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
> shost->host_failed, tries);
> }
>
> -int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> struct domain_device *dev = sdev_to_domain_dev(sdev);
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 60bcc6df97a9..4c513651c95f 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
> mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
> }
>
> -static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> if (sdebug_verbose) {
> if (0x1261 == cmd)
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index a25a07a0b7f0..498bdfbbebfa 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -5572,7 +5572,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
> return rc;
> }
>
> -static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> int rc;
> struct pqi_ctrl_info *ctrl_info;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 38c95d66ab12..263ca4f05c24 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1123,10 +1123,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
> extern void ata_host_detach(struct ata_host *host);
> extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
> extern int ata_scsi_detect(struct scsi_host_template *sht);
> -extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
> extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
> - int cmd, void __user *arg);
> + unsigned int cmd, void __user *arg);
> extern void ata_sas_port_destroy(struct ata_port *);
> extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
> struct ata_port_info *, struct Scsi_Host *);
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 3de3b10da19a..70461264d6e7 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -709,7 +709,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);
>
> extern void sas_target_destroy(struct scsi_target *);
> extern int sas_slave_alloc(struct scsi_device *);
> -extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> extern int sas_drain_work(struct sas_ha_struct *ha);
>
> extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 5ea06d310a25..5c1df26e1d4f 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -65,7 +65,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
>
> #ifdef CONFIG_COMPAT
> @@ -75,7 +76,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> #endif
>
> /*
> --
> 2.19.1
>

Gentle ping for review, it would be nice to get this into 4.21/5.0 since
I sent it out almost two months ago.

Nathan

2019-01-14 04:46:35

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v3] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers,
which don't find into type 'int', which is used for the cmd paremeter in
the ioctls in scsi_host_template. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful. However, looking at
the rest of the kernel tree, all ioctls use an 'unsigned int' for the
cmd parameter, which will fit all of the _IOC values in the scsi/ata
subsystems.

Make that change because none of the ioctls expect to take a negative
value, it brings the ioctls inline with the reset of the kernel, and it
removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
---

v2 -> v3:

* Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
from decode_hioctl (uncovered via a powernv_defconfig build).

v1 -> v2:

* Fix build breakage in cxlflash driver from forgetting to update
prototype (thanks to 0day for catching it).

I would appreciate some review so I could get this accepted...

drivers/ata/libata-scsi.c | 5 +++--
drivers/scsi/aacraid/aachba.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 4 ++--
drivers/scsi/aacraid/commctrl.c | 2 +-
drivers/scsi/aacraid/linit.c | 6 ++++--
drivers/scsi/cxlflash/common.h | 3 ++-
drivers/scsi/cxlflash/main.c | 2 +-
drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
drivers/scsi/esas2r/esas2r.h | 4 ++--
drivers/scsi/esas2r/esas2r_ioctl.c | 13 ++++++-------
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/hpsa.c | 15 +++++++++------
drivers/scsi/ipr.c | 3 ++-
drivers/scsi/libsas/sas_scsi_host.c | 2 +-
drivers/scsi/scsi_debug.c | 3 ++-
drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
include/linux/libata.h | 5 +++--
include/scsi/libsas.h | 3 ++-
include/scsi/scsi_host.h | 6 ++++--
19 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3d4887d0e84a..6291f1dbf342 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
}

int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
unsigned long val;
int rc = -EINVAL;
@@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
}
EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);

-int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
+int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
+ void __user *arg)
{
return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
scsidev, cmd, arg);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 75ab5ff6b78c..6085aa087a2f 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
}
}

-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
switch (cmd) {
case FSACTL_QUERY_DISK:
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 3291d1c16864..1df5171594b8 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
int aac_get_config_status(struct aac_dev *dev, int commit_flag);
int aac_get_containers(struct aac_dev *dev);
int aac_scsi_cmd(struct scsi_cmnd *cmd);
-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
#ifndef shost_to_class
#define shost_to_class(shost) &shost->shost_dev
#endif
ssize_t aac_get_serial_number(struct device *dev, char *buf);
-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
int aac_rx_init(struct aac_dev *dev);
int aac_rkt_init(struct aac_dev *dev);
int aac_nark_init(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index e2899ff7913e..f0ff40332753 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
return retval;
}

-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
int status;

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 634ddb90e7aa..17bcad701cf0 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
NULL,
};

-static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
+static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
@@ -1206,7 +1207,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
return ret;
}

-static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 8908a20065c8..4d90106fcb37 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
void cxlflash_free_errpage(void);
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index bfa13e3b191c..a0ea2dea7518 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
*
* Return: A string identifying the decoded host ioctl.
*/
-static char *decode_hioctl(int cmd)
+static char *decode_hioctl(unsigned int cmd)
{
switch (cmd) {
case HT_CXLFLASH_LUN_PROVISION:
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index acac6152f50b..1a94a469051e 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
*
* Return: A string identifying the decoded ioctl.
*/
-static char *decode_ioctl(int cmd)
+static char *decode_ioctl(unsigned int cmd)
{
switch (cmd) {
case DK_CXLFLASH_ATTACH:
@@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
*
* Return: 0 on success, -errno on failure
*/
-static int ioctl_common(struct scsi_device *sdev, int cmd)
+static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
{
struct cxlflash_cfg *cfg = shost_priv(sdev->host);
struct device *dev = &cfg->dev->dev;
@@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
*
* Return: 0 on success, -errno on failure
*/
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
typedef int (*sioctl) (struct scsi_device *, void *);

@@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
}

if (unlikely(copy_from_user(&buf, arg, size))) {
- dev_err(dev, "%s: copy_from_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
goto cxlflash_ioctl_exit;
@@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
rc = do_ioctl(sdev, (void *)&buf);
if (likely(!rc))
if (unlikely(copy_to_user(arg, &buf, size))) {
- dev_err(dev, "%s: copy_to_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
}
diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 858c3b33db78..7f43b95f4e94 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -965,8 +965,8 @@ struct esas2r_adapter {
const char *esas2r_info(struct Scsi_Host *);
int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
-int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
+int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
u8 handle_hba_ioctl(struct esas2r_adapter *a,
struct atto_ioctl *ioctl_hba);
int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
index 34bcc8c04ff4..916b9adf4921 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,


/* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
{
struct atto_express_ioctl *ioctl = NULL;
struct esas2r_adapter *a;
@@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
if (IS_ERR(ioctl)) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler access_ok failed for cmd %d, "
+ "ioctl_handler access_ok failed for cmd %u, "
"address %p", cmd,
arg);
return PTR_ERR(ioctl);
@@ -1493,7 +1493,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl_done:

if (err < 0) {
- esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
+ esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
cmd);

switch (err) {
@@ -1518,9 +1518,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
if (err != 0) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler copy_to_user didn't copy "
- "everything (err %d, cmd %d)", err,
- cmd);
+ "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
+ err, cmd);
kfree(ioctl);

return -EFAULT;
@@ -1531,7 +1530,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
return 0;
}

-int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
+int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
{
return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
}
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 64397d441bae..fdbda5c05aa0 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -623,7 +623,7 @@ static int esas2r_proc_major;
long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
- (int)cmd, (void __user *)arg);
+ cmd, (void __user *)arg);
}

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ff67ef5d5347..28cfd3d01c5a 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);

#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
void __user *arg);
#endif

@@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)

#ifdef CONFIG_COMPAT

-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
@@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
}

static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}

-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index d1b4025a4503..6d053e220153 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
* Return value:
* 0 on success / other on failure
**/
-static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct ipr_resource_entry *res;

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index c43a00a9d819..b775445892af 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
shost->host_failed, tries);
}

-int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 661512bec3ac..e0f949b7d947 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
}

-static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
if (sdebug_verbose) {
if (0x1261 == cmd)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index 40f58238ce4a..8baaf35da935 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

-static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 68133842e6d7..c9419c05a90a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
extern void ata_host_detach(struct ata_host *host);
extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
extern int ata_scsi_detect(struct scsi_host_template *sht);
-extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
- int cmd, void __user *arg);
+ unsigned int cmd, void __user *arg);
extern void ata_sas_port_destroy(struct ata_port *);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 3de3b10da19a..70461264d6e7 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -709,7 +709,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);

extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
-extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
extern int sas_drain_work(struct sas_ha_struct *ha);

extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 6ca954e9f752..4047d68d1b08 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -60,7 +60,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);


#ifdef CONFIG_COMPAT
@@ -70,7 +71,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
#endif

/*
--
2.20.1


2019-01-14 04:55:57

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH v3] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On 1/13/19 8:42 PM, Nathan Chancellor wrote:
> @@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> if (IS_ERR(ioctl)) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler access_ok failed for cmd %d, "
> + "ioctl_handler access_ok failed for cmd %u, "
> "address %p", cmd,
> arg);
> return PTR_ERR(ioctl);

If you have to repost this patch, please use that opportunity to join
the above split strings. Additionally, since this patch touches much
more SCSI code than ATA code I think it should have been sent to Martin
instead of to Jens. Anyway:

Reviewed-by: Bart Van Assche <[email protected]>


2019-01-14 04:59:19

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v3] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Sun, Jan 13, 2019 at 08:54:35PM -0800, Bart Van Assche wrote:
> On 1/13/19 8:42 PM, Nathan Chancellor wrote:
> > @@ -1292,7 +1292,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> > ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> > if (IS_ERR(ioctl)) {
> > esas2r_log(ESAS2R_LOG_WARN,
> > - "ioctl_handler access_ok failed for cmd %d, "
> > + "ioctl_handler access_ok failed for cmd %u, "
> > "address %p", cmd,
> > arg);
> > return PTR_ERR(ioctl);
>
> If you have to repost this patch, please use that opportunity to join the
> above split strings. Additionally, since this patch touches much more SCSI

Sorry, I should have caught that. If I have to post again, I will fix
that.

> code than ATA code I think it should have been sent to Martin instead of to
> Jens. Anyway:
>

I had Jens, Martin, and James on the 'to' line, with all of the other
maintainers of the individual files on cc. If I should have done it
differently, please let me know.

> Reviewed-by: Bart Van Assche <[email protected]>
>

Thank you, I will carry this forward if I need to.

Nathan

2019-01-26 07:55:19

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers,
which don't find into type 'int', which is used for the cmd paremeter in
the ioctls in scsi_host_template. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful. However, looking at
the rest of the kernel tree, all ioctls use an 'unsigned int' for the
cmd parameter, which will fit all of the _IOC values in the scsi/ata
subsystems.

Make that change because none of the ioctls expect to take a negative
value, it brings the ioctls inline with the reset of the kernel, and it
removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
---

v3 -> v4:

* Repost with Bart's suggested change and his reviewed by (mainly as an
excuse to repost with the below comment).

v2 -> v3:

* Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
from decode_hioctl (uncovered via a powernv_defconfig build).

v1 -> v2:

* Fix build breakage in cxlflash driver from forgetting to update
prototype (thanks to 0day for catching it).

I apologize for resending this so soon after v3 but it has been three
months since I sent this patch and I have yet to receive a single
comment from ANY of the maintainers that './scripts/get_maintainer.pl'
spits out for this patch so it feels like I am sending it into the void.

I understand you all are busy people and such and maybe I am being
impatient but these warnings are rather spammy and we're trying to get
that category cleaned up as quick as possible so that new warnings are
easier to spot. I'd appreciate any and all reviews/acks/testing that I
can get on this patch (especially since it is so simple) so it can be
accepted and merged into mainline.

drivers/ata/libata-scsi.c | 5 +++--
drivers/scsi/aacraid/aachba.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 4 ++--
drivers/scsi/aacraid/commctrl.c | 2 +-
drivers/scsi/aacraid/linit.c | 6 ++++--
drivers/scsi/cxlflash/common.h | 3 ++-
drivers/scsi/cxlflash/main.c | 2 +-
drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
drivers/scsi/esas2r/esas2r.h | 4 ++--
drivers/scsi/esas2r/esas2r_ioctl.c | 16 +++++++---------
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/hpsa.c | 15 +++++++++------
drivers/scsi/ipr.c | 3 ++-
drivers/scsi/libsas/sas_scsi_host.c | 2 +-
drivers/scsi/scsi_debug.c | 3 ++-
drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
include/linux/libata.h | 5 +++--
include/scsi/libsas.h | 3 ++-
include/scsi/scsi_host.h | 6 ++++--
19 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3d4887d0e84a..6291f1dbf342 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
}

int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
unsigned long val;
int rc = -EINVAL;
@@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
}
EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);

-int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
+int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
+ void __user *arg)
{
return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
scsidev, cmd, arg);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 75ab5ff6b78c..6085aa087a2f 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
}
}

-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
switch (cmd) {
case FSACTL_QUERY_DISK:
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 3291d1c16864..1df5171594b8 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
int aac_get_config_status(struct aac_dev *dev, int commit_flag);
int aac_get_containers(struct aac_dev *dev);
int aac_scsi_cmd(struct scsi_cmnd *cmd);
-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
#ifndef shost_to_class
#define shost_to_class(shost) &shost->shost_dev
#endif
ssize_t aac_get_serial_number(struct device *dev, char *buf);
-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
int aac_rx_init(struct aac_dev *dev);
int aac_rkt_init(struct aac_dev *dev);
int aac_nark_init(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index e2899ff7913e..f0ff40332753 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
return retval;
}

-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
int status;

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 7e56a11836c1..cbd955bb8674 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
NULL,
};

-static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
+static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
@@ -1206,7 +1207,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
return ret;
}

-static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 8908a20065c8..4d90106fcb37 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
void cxlflash_free_errpage(void);
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index bfa13e3b191c..a0ea2dea7518 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
*
* Return: A string identifying the decoded host ioctl.
*/
-static char *decode_hioctl(int cmd)
+static char *decode_hioctl(unsigned int cmd)
{
switch (cmd) {
case HT_CXLFLASH_LUN_PROVISION:
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index acac6152f50b..1a94a469051e 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
*
* Return: A string identifying the decoded ioctl.
*/
-static char *decode_ioctl(int cmd)
+static char *decode_ioctl(unsigned int cmd)
{
switch (cmd) {
case DK_CXLFLASH_ATTACH:
@@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
*
* Return: 0 on success, -errno on failure
*/
-static int ioctl_common(struct scsi_device *sdev, int cmd)
+static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
{
struct cxlflash_cfg *cfg = shost_priv(sdev->host);
struct device *dev = &cfg->dev->dev;
@@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
*
* Return: 0 on success, -errno on failure
*/
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
typedef int (*sioctl) (struct scsi_device *, void *);

@@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
}

if (unlikely(copy_from_user(&buf, arg, size))) {
- dev_err(dev, "%s: copy_from_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
goto cxlflash_ioctl_exit;
@@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
rc = do_ioctl(sdev, (void *)&buf);
if (likely(!rc))
if (unlikely(copy_to_user(arg, &buf, size))) {
- dev_err(dev, "%s: copy_to_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
}
diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 858c3b33db78..7f43b95f4e94 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -965,8 +965,8 @@ struct esas2r_adapter {
const char *esas2r_info(struct Scsi_Host *);
int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
-int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
+int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
u8 handle_hba_ioctl(struct esas2r_adapter *a,
struct atto_ioctl *ioctl_hba);
int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
index 34bcc8c04ff4..3d130523c288 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,


/* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
{
struct atto_express_ioctl *ioctl = NULL;
struct esas2r_adapter *a;
@@ -1292,9 +1292,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
if (IS_ERR(ioctl)) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler access_ok failed for cmd %d, "
- "address %p", cmd,
- arg);
+ "ioctl_handler access_ok failed for cmd %u, address %p",
+ cmd, arg);
return PTR_ERR(ioctl);
}

@@ -1493,7 +1492,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl_done:

if (err < 0) {
- esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
+ esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
cmd);

switch (err) {
@@ -1518,9 +1517,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
if (err != 0) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler copy_to_user didn't copy "
- "everything (err %d, cmd %d)", err,
- cmd);
+ "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
+ err, cmd);
kfree(ioctl);

return -EFAULT;
@@ -1531,7 +1529,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
return 0;
}

-int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
+int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
{
return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
}
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 64397d441bae..fdbda5c05aa0 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -623,7 +623,7 @@ static int esas2r_proc_major;
long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
- (int)cmd, (void __user *)arg);
+ cmd, (void __user *)arg);
}

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ff67ef5d5347..28cfd3d01c5a 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);

#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
void __user *arg);
#endif

@@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)

#ifdef CONFIG_COMPAT

-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
@@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
}

static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}

-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index d1b4025a4503..6d053e220153 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
* Return value:
* 0 on success / other on failure
**/
-static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct ipr_resource_entry *res;

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index c43a00a9d819..b775445892af 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
shost->host_failed, tries);
}

-int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 661512bec3ac..e0f949b7d947 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
}

-static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
if (sdebug_verbose) {
if (0x1261 == cmd)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index f564af8949e8..5d9ccbab7581 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

-static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 68133842e6d7..c9419c05a90a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
extern void ata_host_detach(struct ata_host *host);
extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
extern int ata_scsi_detect(struct scsi_host_template *sht);
-extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
- int cmd, void __user *arg);
+ unsigned int cmd, void __user *arg);
extern void ata_sas_port_destroy(struct ata_port *);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 857086cf7ebf..56b2dba7d911 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -707,7 +707,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);

extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
-extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
extern int sas_drain_work(struct sas_ha_struct *ha);

extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 6ca954e9f752..4047d68d1b08 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -60,7 +60,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);


#ifdef CONFIG_COMPAT
@@ -70,7 +71,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
#endif

/*
--
2.20.1


2019-01-26 09:21:37

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Fri, Jan 25, 2019 at 11:53 PM Nathan Chancellor
<[email protected]> wrote:
>
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't find into type 'int', which is used for the cmd paremeter in
> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect to take a negative
> value, it brings the ioctls inline with the reset of the kernel, and it
> removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>

Thanks for following up and pursuing this patch, Nathan. I also
eagerly await it landing. Sorry for not testing it sooner.

Comparing allyesconfig x86_64 builds before/after your patch:
$ grep Wswitch log.txt | wc -l
17
$ grep Wswitch log2.txt | wc -l
0

So from a warnings perspective
Tested-by: Nick Desaulniers <[email protected]>
I also verified that they did not introduce new warnings (for allyesconfig).

> ---
>
> v3 -> v4:
>
> * Repost with Bart's suggested change and his reviewed by (mainly as an
> excuse to repost with the below comment).
>
> v2 -> v3:
>
> * Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
> from decode_hioctl (uncovered via a powernv_defconfig build).
>
> v1 -> v2:
>
> * Fix build breakage in cxlflash driver from forgetting to update
> prototype (thanks to 0day for catching it).
>
> I apologize for resending this so soon after v3 but it has been three
> months since I sent this patch and I have yet to receive a single
> comment from ANY of the maintainers that './scripts/get_maintainer.pl'
> spits out for this patch so it feels like I am sending it into the void.
>
> I understand you all are busy people and such and maybe I am being
> impatient but these warnings are rather spammy and we're trying to get
> that category cleaned up as quick as possible so that new warnings are
> easier to spot. I'd appreciate any and all reviews/acks/testing that I
> can get on this patch (especially since it is so simple) so it can be
> accepted and merged into mainline.
>
> drivers/ata/libata-scsi.c | 5 +++--
> drivers/scsi/aacraid/aachba.c | 2 +-
> drivers/scsi/aacraid/aacraid.h | 4 ++--
> drivers/scsi/aacraid/commctrl.c | 2 +-
> drivers/scsi/aacraid/linit.c | 6 ++++--
> drivers/scsi/cxlflash/common.h | 3 ++-
> drivers/scsi/cxlflash/main.c | 2 +-
> drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
> drivers/scsi/esas2r/esas2r.h | 4 ++--
> drivers/scsi/esas2r/esas2r_ioctl.c | 16 +++++++---------
> drivers/scsi/esas2r/esas2r_main.c | 2 +-
> drivers/scsi/hpsa.c | 15 +++++++++------
> drivers/scsi/ipr.c | 3 ++-
> drivers/scsi/libsas/sas_scsi_host.c | 2 +-
> drivers/scsi/scsi_debug.c | 3 ++-
> drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
> include/linux/libata.h | 5 +++--
> include/scsi/libsas.h | 3 ++-
> include/scsi/scsi_host.h | 6 ++++--
> 19 files changed, 54 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3d4887d0e84a..6291f1dbf342 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
> }
>
> int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> unsigned long val;
> int rc = -EINVAL;
> @@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> }
> EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);
>
> -int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
> +int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
> + void __user *arg)
> {
> return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
> scsidev, cmd, arg);
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 75ab5ff6b78c..6085aa087a2f 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
> }
> }
>
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> switch (cmd) {
> case FSACTL_QUERY_DISK:
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 3291d1c16864..1df5171594b8 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
> int aac_get_config_status(struct aac_dev *dev, int commit_flag);
> int aac_get_containers(struct aac_dev *dev);
> int aac_scsi_cmd(struct scsi_cmnd *cmd);
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> #ifndef shost_to_class
> #define shost_to_class(shost) &shost->shost_dev
> #endif
> ssize_t aac_get_serial_number(struct device *dev, char *buf);
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> int aac_rx_init(struct aac_dev *dev);
> int aac_rkt_init(struct aac_dev *dev);
> int aac_nark_init(struct aac_dev *dev);
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index e2899ff7913e..f0ff40332753 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
> return retval;
> }
>
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> int status;
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 7e56a11836c1..cbd955bb8674 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
> NULL,
> };
>
> -static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> +static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> @@ -1206,7 +1207,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
> return ret;
> }
>
> -static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 8908a20065c8..4d90106fcb37 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
> void cxlflash_list_init(void);
> void cxlflash_term_global_luns(void);
> void cxlflash_free_errpage(void);
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
> int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
> void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index bfa13e3b191c..a0ea2dea7518 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
> *
> * Return: A string identifying the decoded host ioctl.
> */
> -static char *decode_hioctl(int cmd)
> +static char *decode_hioctl(unsigned int cmd)
> {
> switch (cmd) {
> case HT_CXLFLASH_LUN_PROVISION:
> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
> index acac6152f50b..1a94a469051e 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
> *
> * Return: A string identifying the decoded ioctl.
> */
> -static char *decode_ioctl(int cmd)
> +static char *decode_ioctl(unsigned int cmd)
> {
> switch (cmd) {
> case DK_CXLFLASH_ATTACH:
> @@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
> *
> * Return: 0 on success, -errno on failure
> */
> -static int ioctl_common(struct scsi_device *sdev, int cmd)
> +static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
> {
> struct cxlflash_cfg *cfg = shost_priv(sdev->host);
> struct device *dev = &cfg->dev->dev;
> @@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
> *
> * Return: 0 on success, -errno on failure
> */
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> typedef int (*sioctl) (struct scsi_device *, void *);
>
> @@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> }
>
> if (unlikely(copy_from_user(&buf, arg, size))) {
> - dev_err(dev, "%s: copy_from_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> goto cxlflash_ioctl_exit;
> @@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> rc = do_ioctl(sdev, (void *)&buf);
> if (likely(!rc))
> if (unlikely(copy_to_user(arg, &buf, size))) {
> - dev_err(dev, "%s: copy_to_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> }
> diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
> index 858c3b33db78..7f43b95f4e94 100644
> --- a/drivers/scsi/esas2r/esas2r.h
> +++ b/drivers/scsi/esas2r/esas2r.h
> @@ -965,8 +965,8 @@ struct esas2r_adapter {
> const char *esas2r_info(struct Scsi_Host *);
> int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
> struct esas2r_sas_nvram *data);
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
> -int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
> +int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
> u8 handle_hba_ioctl(struct esas2r_adapter *a,
> struct atto_ioctl *ioctl_hba);
> int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 34bcc8c04ff4..3d130523c288 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
>
>
> /* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
> {
> struct atto_express_ioctl *ioctl = NULL;
> struct esas2r_adapter *a;
> @@ -1292,9 +1292,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> if (IS_ERR(ioctl)) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler access_ok failed for cmd %d, "
> - "address %p", cmd,
> - arg);
> + "ioctl_handler access_ok failed for cmd %u, address %p",
> + cmd, arg);
> return PTR_ERR(ioctl);
> }
>
> @@ -1493,7 +1492,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl_done:
>
> if (err < 0) {
> - esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
> + esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
> cmd);
>
> switch (err) {
> @@ -1518,9 +1517,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
> if (err != 0) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler copy_to_user didn't copy "
> - "everything (err %d, cmd %d)", err,
> - cmd);
> + "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
> + err, cmd);
> kfree(ioctl);
>
> return -EFAULT;
> @@ -1531,7 +1529,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> return 0;
> }
>
> -int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
> +int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
> {
> return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
> }
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index 64397d441bae..fdbda5c05aa0 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -623,7 +623,7 @@ static int esas2r_proc_major;
> long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
> return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
> - (int)cmd, (void __user *)arg);
> + cmd, (void __user *)arg);
> }
>
> static void __exit esas2r_exit(void)
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index ff67ef5d5347..28cfd3d01c5a 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -251,10 +251,11 @@ static int number_of_controllers;
>
> static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
> static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
> #ifdef CONFIG_COMPAT
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> void __user *arg);
> #endif
>
> @@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)
>
> #ifdef CONFIG_COMPAT
>
> -static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> +static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
> void __user *arg)
> {
> IOCTL32_Command_struct __user *arg32 =
> @@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> }
>
> static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> BIG_IOCTL32_Command_struct __user *arg32 =
> (BIG_IOCTL32_Command_struct __user *) arg;
> @@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> return err;
> }
>
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> switch (cmd) {
> case CCISS_GETPCIINFO:
> @@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
> /*
> * ioctl
> */
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> struct ctlr_info *h;
> void __user *argp = (void __user *)arg;
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index d1b4025a4503..6d053e220153 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
> * Return value:
> * 0 on success / other on failure
> **/
> -static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct ipr_resource_entry *res;
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index c43a00a9d819..b775445892af 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
> shost->host_failed, tries);
> }
>
> -int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> struct domain_device *dev = sdev_to_domain_dev(sdev);
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 661512bec3ac..e0f949b7d947 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
> mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
> }
>
> -static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> if (sdebug_verbose) {
> if (0x1261 == cmd)
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index f564af8949e8..5d9ccbab7581 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
> return rc;
> }
>
> -static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> int rc;
> struct pqi_ctrl_info *ctrl_info;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 68133842e6d7..c9419c05a90a 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
> extern void ata_host_detach(struct ata_host *host);
> extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
> extern int ata_scsi_detect(struct scsi_host_template *sht);
> -extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
> extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
> - int cmd, void __user *arg);
> + unsigned int cmd, void __user *arg);
> extern void ata_sas_port_destroy(struct ata_port *);
> extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
> struct ata_port_info *, struct Scsi_Host *);
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 857086cf7ebf..56b2dba7d911 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -707,7 +707,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);
>
> extern void sas_target_destroy(struct scsi_target *);
> extern int sas_slave_alloc(struct scsi_device *);
> -extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> extern int sas_drain_work(struct sas_ha_struct *ha);
>
> extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 6ca954e9f752..4047d68d1b08 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -60,7 +60,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
>
> #ifdef CONFIG_COMPAT
> @@ -70,7 +71,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> #endif
>
> /*
> --
> 2.20.1
>


--
Thanks,
~Nick Desaulniers

2019-01-28 16:55:43

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Mon, Jan 28, 2019 at 04:16:34PM +0000, [email protected] wrote:
>
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers, which don't find into type 'int', which is used for the cmd paremeter in the ioctls in scsi_host_template. My research into how GCC and Clang are handling this at a low level didn't prove fruitful. However, looking at the rest of the kernel tree, all ioctls use an 'unsigned int' for the cmd parameter, which will fit all of the _IOC values in the scsi/ata subsystems.
>
> Make that change because none of the ioctls expect to take a negative value, it brings the ioctls inline with the reset of the kernel, and it removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
>
> static void __exit esas2r_exit(void)
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index ff67ef5d5347..28cfd3d01c5a 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -251,10 +251,11 @@ static int number_of_controllers;
>
> Acked-by: Don Brace <[email protected]>
>
>
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index f564af8949e8..5d9ccbab7581 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
> return rc;
> }
>
> Acked-by: Don Brace <[email protected]>
>
>

Thank you for the reply and the review, I really appreciate it :)

Nathan

2019-01-28 16:59:21

by Don Brace

[permalink] [raw]
Subject: RE: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template


Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers, which don't find into type 'int', which is used for the cmd paremeter in the ioctls in scsi_host_template. My research into how GCC and Clang are handling this at a low level didn't prove fruitful. However, looking at the rest of the kernel tree, all ioctls use an 'unsigned int' for the cmd parameter, which will fit all of the _IOC values in the scsi/ata subsystems.

Make that change because none of the ioctls expect to take a negative value, it brings the ioctls inline with the reset of the kernel, and it removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index ff67ef5d5347..28cfd3d01c5a 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

Acked-by: Don Brace <[email protected]>


diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index f564af8949e8..5d9ccbab7581 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

Acked-by: Don Brace <[email protected]>



2019-02-05 19:03:09

by Grove, Bradley

[permalink] [raw]
Subject: Re: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template


Acked-by: Bradley Grove <[email protected]>

On 1/26/2019 2:52 AM, Nathan Chancellor wrote:
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't find into type 'int', which is used for the cmd paremeter in
> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect to take a negative
> value, it brings the ioctls inline with the reset of the kernel, and it
> removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> ---
>
> v3 -> v4:
>
> * Repost with Bart's suggested change and his reviewed by (mainly as an
> excuse to repost with the below comment).
>
> v2 -> v3:
>
> * Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
> from decode_hioctl (uncovered via a powernv_defconfig build).
>
> v1 -> v2:
>
> * Fix build breakage in cxlflash driver from forgetting to update
> prototype (thanks to 0day for catching it).
>
> I apologize for resending this so soon after v3 but it has been three
> months since I sent this patch and I have yet to receive a single
> comment from ANY of the maintainers that './scripts/get_maintainer.pl'
> spits out for this patch so it feels like I am sending it into the void.
>
> I understand you all are busy people and such and maybe I am being
> impatient but these warnings are rather spammy and we're trying to get
> that category cleaned up as quick as possible so that new warnings are
> easier to spot. I'd appreciate any and all reviews/acks/testing that I
> can get on this patch (especially since it is so simple) so it can be
> accepted and merged into mainline.
>
> drivers/ata/libata-scsi.c | 5 +++--
> drivers/scsi/aacraid/aachba.c | 2 +-
> drivers/scsi/aacraid/aacraid.h | 4 ++--
> drivers/scsi/aacraid/commctrl.c | 2 +-
> drivers/scsi/aacraid/linit.c | 6 ++++--
> drivers/scsi/cxlflash/common.h | 3 ++-
> drivers/scsi/cxlflash/main.c | 2 +-
> drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
> drivers/scsi/esas2r/esas2r.h | 4 ++--
> drivers/scsi/esas2r/esas2r_ioctl.c | 16 +++++++---------
> drivers/scsi/esas2r/esas2r_main.c | 2 +-
> drivers/scsi/hpsa.c | 15 +++++++++------
> drivers/scsi/ipr.c | 3 ++-
> drivers/scsi/libsas/sas_scsi_host.c | 2 +-
> drivers/scsi/scsi_debug.c | 3 ++-
> drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
> include/linux/libata.h | 5 +++--
> include/scsi/libsas.h | 3 ++-
> include/scsi/scsi_host.h | 6 ++++--
> 19 files changed, 54 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3d4887d0e84a..6291f1dbf342 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
> }
>
> int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> unsigned long val;
> int rc = -EINVAL;
> @@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> }
> EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);
>
> -int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
> +int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
> + void __user *arg)
> {
> return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
> scsidev, cmd, arg);
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 75ab5ff6b78c..6085aa087a2f 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
> }
> }
>
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> switch (cmd) {
> case FSACTL_QUERY_DISK:
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 3291d1c16864..1df5171594b8 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
> int aac_get_config_status(struct aac_dev *dev, int commit_flag);
> int aac_get_containers(struct aac_dev *dev);
> int aac_scsi_cmd(struct scsi_cmnd *cmd);
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> #ifndef shost_to_class
> #define shost_to_class(shost) &shost->shost_dev
> #endif
> ssize_t aac_get_serial_number(struct device *dev, char *buf);
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> int aac_rx_init(struct aac_dev *dev);
> int aac_rkt_init(struct aac_dev *dev);
> int aac_nark_init(struct aac_dev *dev);
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index e2899ff7913e..f0ff40332753 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
> return retval;
> }
>
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> int status;
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 7e56a11836c1..cbd955bb8674 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
> NULL,
> };
>
> -static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> +static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> @@ -1206,7 +1207,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
> return ret;
> }
>
> -static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 8908a20065c8..4d90106fcb37 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
> void cxlflash_list_init(void);
> void cxlflash_term_global_luns(void);
> void cxlflash_free_errpage(void);
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
> int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
> void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index bfa13e3b191c..a0ea2dea7518 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
> *
> * Return: A string identifying the decoded host ioctl.
> */
> -static char *decode_hioctl(int cmd)
> +static char *decode_hioctl(unsigned int cmd)
> {
> switch (cmd) {
> case HT_CXLFLASH_LUN_PROVISION:
> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
> index acac6152f50b..1a94a469051e 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
> *
> * Return: A string identifying the decoded ioctl.
> */
> -static char *decode_ioctl(int cmd)
> +static char *decode_ioctl(unsigned int cmd)
> {
> switch (cmd) {
> case DK_CXLFLASH_ATTACH:
> @@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
> *
> * Return: 0 on success, -errno on failure
> */
> -static int ioctl_common(struct scsi_device *sdev, int cmd)
> +static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
> {
> struct cxlflash_cfg *cfg = shost_priv(sdev->host);
> struct device *dev = &cfg->dev->dev;
> @@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
> *
> * Return: 0 on success, -errno on failure
> */
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> typedef int (*sioctl) (struct scsi_device *, void *);
>
> @@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> }
>
> if (unlikely(copy_from_user(&buf, arg, size))) {
> - dev_err(dev, "%s: copy_from_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> goto cxlflash_ioctl_exit;
> @@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> rc = do_ioctl(sdev, (void *)&buf);
> if (likely(!rc))
> if (unlikely(copy_to_user(arg, &buf, size))) {
> - dev_err(dev, "%s: copy_to_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> }
> diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
> index 858c3b33db78..7f43b95f4e94 100644
> --- a/drivers/scsi/esas2r/esas2r.h
> +++ b/drivers/scsi/esas2r/esas2r.h
> @@ -965,8 +965,8 @@ struct esas2r_adapter {
> const char *esas2r_info(struct Scsi_Host *);
> int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
> struct esas2r_sas_nvram *data);
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
> -int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
> +int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
> u8 handle_hba_ioctl(struct esas2r_adapter *a,
> struct atto_ioctl *ioctl_hba);
> int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 34bcc8c04ff4..3d130523c288 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
>
>
> /* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
> {
> struct atto_express_ioctl *ioctl = NULL;
> struct esas2r_adapter *a;
> @@ -1292,9 +1292,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> if (IS_ERR(ioctl)) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler access_ok failed for cmd %d, "
> - "address %p", cmd,
> - arg);
> + "ioctl_handler access_ok failed for cmd %u, address %p",
> + cmd, arg);
> return PTR_ERR(ioctl);
> }
>
> @@ -1493,7 +1492,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl_done:
>
> if (err < 0) {
> - esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
> + esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
> cmd);
>
> switch (err) {
> @@ -1518,9 +1517,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
> if (err != 0) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler copy_to_user didn't copy "
> - "everything (err %d, cmd %d)", err,
> - cmd);
> + "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
> + err, cmd);
> kfree(ioctl);
>
> return -EFAULT;
> @@ -1531,7 +1529,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> return 0;
> }
>
> -int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
> +int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
> {
> return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
> }
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index 64397d441bae..fdbda5c05aa0 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -623,7 +623,7 @@ static int esas2r_proc_major;
> long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
> return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
> - (int)cmd, (void __user *)arg);
> + cmd, (void __user *)arg);
> }
>
> static void __exit esas2r_exit(void)
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index ff67ef5d5347..28cfd3d01c5a 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -251,10 +251,11 @@ static int number_of_controllers;
>
> static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
> static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
> #ifdef CONFIG_COMPAT
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> void __user *arg);
> #endif
>
> @@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)
>
> #ifdef CONFIG_COMPAT
>
> -static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> +static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
> void __user *arg)
> {
> IOCTL32_Command_struct __user *arg32 =
> @@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> }
>
> static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> BIG_IOCTL32_Command_struct __user *arg32 =
> (BIG_IOCTL32_Command_struct __user *) arg;
> @@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> return err;
> }
>
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> switch (cmd) {
> case CCISS_GETPCIINFO:
> @@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
> /*
> * ioctl
> */
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> struct ctlr_info *h;
> void __user *argp = (void __user *)arg;
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index d1b4025a4503..6d053e220153 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
> * Return value:
> * 0 on success / other on failure
> **/
> -static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct ipr_resource_entry *res;
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index c43a00a9d819..b775445892af 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
> shost->host_failed, tries);
> }
>
> -int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> struct domain_device *dev = sdev_to_domain_dev(sdev);
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 661512bec3ac..e0f949b7d947 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -836,7 +836,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
> mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
> }
>
> -static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> if (sdebug_verbose) {
> if (0x1261 == cmd)
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index f564af8949e8..5d9ccbab7581 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
> return rc;
> }
>
> -static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> int rc;
> struct pqi_ctrl_info *ctrl_info;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 68133842e6d7..c9419c05a90a 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
> extern void ata_host_detach(struct ata_host *host);
> extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
> extern int ata_scsi_detect(struct scsi_host_template *sht);
> -extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
> extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
> - int cmd, void __user *arg);
> + unsigned int cmd, void __user *arg);
> extern void ata_sas_port_destroy(struct ata_port *);
> extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
> struct ata_port_info *, struct Scsi_Host *);
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 857086cf7ebf..56b2dba7d911 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -707,7 +707,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);
>
> extern void sas_target_destroy(struct scsi_target *);
> extern int sas_slave_alloc(struct scsi_device *);
> -extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> extern int sas_drain_work(struct sas_ha_struct *ha);
>
> extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 6ca954e9f752..4047d68d1b08 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -60,7 +60,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
>
> #ifdef CONFIG_COMPAT
> @@ -70,7 +71,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> #endif
>
> /*
>


This electronic transmission and any attachments hereto are intended only for the use of the individual or entity to which it is addressed and may contain confidential information belonging to ATTO Technology, Inc. If you have reason to believe that you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or the taking of any action in reliance on the contents of this electronic transmission is strictly prohibited. If you have reason to believe that you have received this transmission in error, please notify ATTO immediately by return e-mail and delete and destroy this communication.

2019-02-07 10:54:27

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v4] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On 26/01/2019 08:52, Nathan Chancellor wrote:

> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't find into type 'int', which is used for the cmd paremeter in
^^^^ ^^^^^^^^^

"which don't fit"
"the cmd parameter"

Regards.

> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect to take a negative
> value, it brings the ioctls inline with the reset of the kernel, and it
> removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> ---
>
> v3 -> v4:
>
> * Repost with Bart's suggested change and his reviewed by (mainly as an
> excuse to repost with the below comment).
>
> v2 -> v3:
>
> * Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
> from decode_hioctl (uncovered via a powernv_defconfig build).
>
> v1 -> v2:
>
> * Fix build breakage in cxlflash driver from forgetting to update
> prototype (thanks to 0day for catching it).
>
> I apologize for resending this so soon after v3 but it has been three
> months since I sent this patch and I have yet to receive a single
> comment from ANY of the maintainers that './scripts/get_maintainer.pl'
> spits out for this patch so it feels like I am sending it into the void.
>
> I understand you all are busy people and such and maybe I am being
> impatient but these warnings are rather spammy and we're trying to get
> that category cleaned up as quick as possible so that new warnings are
> easier to spot. I'd appreciate any and all reviews/acks/testing that I
> can get on this patch (especially since it is so simple) so it can be
> accepted and merged into mainline.

2019-02-07 16:12:07

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v5] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

Clang warns several times in the scsi subsystem (trimmed for brevity):

drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
case CCISS_GETBUSTYPES:
^
drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
case CCISS_GETHEARTBEAT:
^

The root cause is that the _IOC macro can generate really large numbers,
which don't fit into type 'int', which is used for the cmd parameter in
the ioctls in scsi_host_template. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful. However, looking at
the rest of the kernel tree, all ioctls use an 'unsigned int' for the
cmd parameter, which will fit all of the _IOC values in the scsi/ata
subsystems.

Make that change because none of the ioctls expect a negative value for
any command, it brings the ioctls inline with the reset of the kernel,
and it removes ambiguity, which is never good when dealing with compilers.

Link: https://github.com/ClangBuiltLinux/linux/issues/85
Link: https://github.com/ClangBuiltLinux/linux/issues/154
Link: https://github.com/ClangBuiltLinux/linux/issues/157
Signed-off-by: Nathan Chancellor <[email protected]>
Acked-by: Bradley Grove <[email protected]>
Acked-by: Don Brace <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
---

v4 -> v5:

* Collect tags.
* Fix mispellings in the commit message, thanks to Marc Gonzalez for the
review.

v3 -> v4:

* Repost with Bart's suggested change and his reviewed by.

v2 -> v3:

* Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
from decode_hioctl (uncovered via a powernv_defconfig build).

v1 -> v2:

* Fix build breakage in cxlflash driver from forgetting to update
prototype (thanks to 0day for catching it).

drivers/ata/libata-scsi.c | 5 +++--
drivers/scsi/aacraid/aachba.c | 2 +-
drivers/scsi/aacraid/aacraid.h | 4 ++--
drivers/scsi/aacraid/commctrl.c | 2 +-
drivers/scsi/aacraid/linit.c | 6 ++++--
drivers/scsi/cxlflash/common.h | 3 ++-
drivers/scsi/cxlflash/main.c | 2 +-
drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
drivers/scsi/esas2r/esas2r.h | 4 ++--
drivers/scsi/esas2r/esas2r_ioctl.c | 16 +++++++---------
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/hpsa.c | 15 +++++++++------
drivers/scsi/ipr.c | 3 ++-
drivers/scsi/libsas/sas_scsi_host.c | 2 +-
drivers/scsi/scsi_debug.c | 3 ++-
drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
include/linux/libata.h | 5 +++--
include/scsi/libsas.h | 3 ++-
include/scsi/scsi_host.h | 6 ++++--
19 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3d4887d0e84a..6291f1dbf342 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
}

int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
unsigned long val;
int rc = -EINVAL;
@@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
}
EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);

-int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
+int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
+ void __user *arg)
{
return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
scsidev, cmd, arg);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 75ab5ff6b78c..6085aa087a2f 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
}
}

-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
switch (cmd) {
case FSACTL_QUERY_DISK:
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 3291d1c16864..1df5171594b8 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
int aac_get_config_status(struct aac_dev *dev, int commit_flag);
int aac_get_containers(struct aac_dev *dev);
int aac_scsi_cmd(struct scsi_cmnd *cmd);
-int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
+int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
#ifndef shost_to_class
#define shost_to_class(shost) &shost->shost_dev
#endif
ssize_t aac_get_serial_number(struct device *dev, char *buf);
-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
int aac_rx_init(struct aac_dev *dev);
int aac_rkt_init(struct aac_dev *dev);
int aac_nark_init(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index e2899ff7913e..f0ff40332753 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
return retval;
}

-int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
+int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
{
int status;

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index e977b459f823..a45f81ec80ce 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
NULL,
};

-static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
+static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
@@ -1205,7 +1206,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
return ret;
}

-static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
if (!capable(CAP_SYS_RAWIO))
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 8908a20065c8..4d90106fcb37 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
void cxlflash_free_errpage(void);
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index c8bad2c093b8..7096810fd222 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
*
* Return: A string identifying the decoded host ioctl.
*/
-static char *decode_hioctl(int cmd)
+static char *decode_hioctl(unsigned int cmd)
{
switch (cmd) {
case HT_CXLFLASH_LUN_PROVISION:
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index acac6152f50b..1a94a469051e 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
*
* Return: A string identifying the decoded ioctl.
*/
-static char *decode_ioctl(int cmd)
+static char *decode_ioctl(unsigned int cmd)
{
switch (cmd) {
case DK_CXLFLASH_ATTACH:
@@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
*
* Return: 0 on success, -errno on failure
*/
-static int ioctl_common(struct scsi_device *sdev, int cmd)
+static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
{
struct cxlflash_cfg *cfg = shost_priv(sdev->host);
struct device *dev = &cfg->dev->dev;
@@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
*
* Return: 0 on success, -errno on failure
*/
-int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
typedef int (*sioctl) (struct scsi_device *, void *);

@@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
}

if (unlikely(copy_from_user(&buf, arg, size))) {
- dev_err(dev, "%s: copy_from_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
goto cxlflash_ioctl_exit;
@@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
rc = do_ioctl(sdev, (void *)&buf);
if (likely(!rc))
if (unlikely(copy_to_user(arg, &buf, size))) {
- dev_err(dev, "%s: copy_to_user() fail "
- "size=%lu cmd=%d (%s) arg=%p\n",
+ dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
__func__, size, cmd, decode_ioctl(cmd), arg);
rc = -EFAULT;
}
diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 858c3b33db78..7f43b95f4e94 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -965,8 +965,8 @@ struct esas2r_adapter {
const char *esas2r_info(struct Scsi_Host *);
int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
struct esas2r_sas_nvram *data);
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
-int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
+int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
u8 handle_hba_ioctl(struct esas2r_adapter *a,
struct atto_ioctl *ioctl_hba);
int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
index 34bcc8c04ff4..3d130523c288 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,


/* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
-int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
+int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
{
struct atto_express_ioctl *ioctl = NULL;
struct esas2r_adapter *a;
@@ -1292,9 +1292,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
if (IS_ERR(ioctl)) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler access_ok failed for cmd %d, "
- "address %p", cmd,
- arg);
+ "ioctl_handler access_ok failed for cmd %u, address %p",
+ cmd, arg);
return PTR_ERR(ioctl);
}

@@ -1493,7 +1492,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
ioctl_done:

if (err < 0) {
- esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
+ esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
cmd);

switch (err) {
@@ -1518,9 +1517,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
if (err != 0) {
esas2r_log(ESAS2R_LOG_WARN,
- "ioctl_handler copy_to_user didn't copy "
- "everything (err %d, cmd %d)", err,
- cmd);
+ "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
+ err, cmd);
kfree(ioctl);

return -EFAULT;
@@ -1531,7 +1529,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
return 0;
}

-int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
+int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
{
return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
}
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 64397d441bae..fdbda5c05aa0 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -623,7 +623,7 @@ static int esas2r_proc_major;
long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
- (int)cmd, (void __user *)arg);
+ cmd, (void __user *)arg);
}

static void __exit esas2r_exit(void)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 5284444fdd10..f044e7d10d63 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -251,10 +251,11 @@ static int number_of_controllers;

static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);

#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
void __user *arg);
#endif

@@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)

#ifdef CONFIG_COMPAT

-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
@@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
}

static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void __user *arg)
+ unsigned int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
return err;
}

-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index d1b4025a4503..6d053e220153 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
* Return value:
* 0 on success / other on failure
**/
-static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
struct ipr_resource_entry *res;

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index c43a00a9d819..b775445892af 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
shost->host_failed, tries);
}

-int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
{
struct domain_device *dev = sdev_to_domain_dev(sdev);

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d4aede324e84..3eb47207c7b0 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -832,7 +832,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
}

-static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
+static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg)
{
if (sdebug_verbose) {
if (0x1261 == cmd)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index f564af8949e8..5d9ccbab7581 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
return rc;
}

-static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
+static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 68133842e6d7..c9419c05a90a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
extern void ata_host_detach(struct ata_host *host);
extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
extern int ata_scsi_detect(struct scsi_host_template *sht);
-extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
+extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
- int cmd, void __user *arg);
+ unsigned int cmd, void __user *arg);
extern void ata_sas_port_destroy(struct ata_port *);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 857086cf7ebf..56b2dba7d911 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -707,7 +707,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);

extern void sas_target_destroy(struct scsi_target *);
extern int sas_slave_alloc(struct scsi_device *);
-extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
+extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
+ void __user *arg);
extern int sas_drain_work(struct sas_ha_struct *ha);

extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 6ca954e9f752..4047d68d1b08 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -60,7 +60,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);


#ifdef CONFIG_COMPAT
@@ -70,7 +71,8 @@ struct scsi_host_template {
*
* Status: OPTIONAL
*/
- int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
+ int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
+ void __user *arg);
#endif

/*
--
2.20.1


2019-02-07 19:18:29

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v5] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Thu, Feb 7, 2019 at 8:09 AM Nathan Chancellor
<[email protected]> wrote:
>
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't fit into type 'int', which is used for the cmd parameter in
> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect a negative value for
> any command, it brings the ioctls inline with the reset of the kernel,
> and it removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Acked-by: Bradley Grove <[email protected]>
> Acked-by: Don Brace <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>

Thanks for resending with the collected tags and small fixups.

James or Martin, can you please pick this up?

> ---
>
> v4 -> v5:
>
> * Collect tags.
> * Fix mispellings in the commit message, thanks to Marc Gonzalez for the
> review.
>
> v3 -> v4:
>
> * Repost with Bart's suggested change and his reviewed by.
>
> v2 -> v3:
>
> * Fix another -Wswitch warning from drivers/scsi/cxlflash/main.c, coming
> from decode_hioctl (uncovered via a powernv_defconfig build).
>
> v1 -> v2:
>
> * Fix build breakage in cxlflash driver from forgetting to update
> prototype (thanks to 0day for catching it).
>
> drivers/ata/libata-scsi.c | 5 +++--
> drivers/scsi/aacraid/aachba.c | 2 +-
> drivers/scsi/aacraid/aacraid.h | 4 ++--
> drivers/scsi/aacraid/commctrl.c | 2 +-
> drivers/scsi/aacraid/linit.c | 6 ++++--
> drivers/scsi/cxlflash/common.h | 3 ++-
> drivers/scsi/cxlflash/main.c | 2 +-
> drivers/scsi/cxlflash/superpipe.c | 12 +++++-------
> drivers/scsi/esas2r/esas2r.h | 4 ++--
> drivers/scsi/esas2r/esas2r_ioctl.c | 16 +++++++---------
> drivers/scsi/esas2r/esas2r_main.c | 2 +-
> drivers/scsi/hpsa.c | 15 +++++++++------
> drivers/scsi/ipr.c | 3 ++-
> drivers/scsi/libsas/sas_scsi_host.c | 2 +-
> drivers/scsi/scsi_debug.c | 3 ++-
> drivers/scsi/smartpqi/smartpqi_init.c | 3 ++-
> include/linux/libata.h | 5 +++--
> include/scsi/libsas.h | 3 ++-
> include/scsi/scsi_host.h | 6 ++++--
> 19 files changed, 54 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3d4887d0e84a..6291f1dbf342 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -778,7 +778,7 @@ static int ata_ioc32(struct ata_port *ap)
> }
>
> int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> unsigned long val;
> int rc = -EINVAL;
> @@ -829,7 +829,8 @@ int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev,
> }
> EXPORT_SYMBOL_GPL(ata_sas_scsi_ioctl);
>
> -int ata_scsi_ioctl(struct scsi_device *scsidev, int cmd, void __user *arg)
> +int ata_scsi_ioctl(struct scsi_device *scsidev, unsigned int cmd,
> + void __user *arg)
> {
> return ata_sas_scsi_ioctl(ata_shost_to_port(scsidev->host),
> scsidev, cmd, arg);
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 75ab5ff6b78c..6085aa087a2f 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -3455,7 +3455,7 @@ static int delete_disk(struct aac_dev *dev, void __user *arg)
> }
> }
>
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg)
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> switch (cmd) {
> case FSACTL_QUERY_DISK:
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 3291d1c16864..1df5171594b8 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2706,12 +2706,12 @@ void aac_set_intx_mode(struct aac_dev *dev);
> int aac_get_config_status(struct aac_dev *dev, int commit_flag);
> int aac_get_containers(struct aac_dev *dev);
> int aac_scsi_cmd(struct scsi_cmnd *cmd);
> -int aac_dev_ioctl(struct aac_dev *dev, int cmd, void __user *arg);
> +int aac_dev_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> #ifndef shost_to_class
> #define shost_to_class(shost) &shost->shost_dev
> #endif
> ssize_t aac_get_serial_number(struct device *dev, char *buf);
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg);
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg);
> int aac_rx_init(struct aac_dev *dev);
> int aac_rkt_init(struct aac_dev *dev);
> int aac_nark_init(struct aac_dev *dev);
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index e2899ff7913e..f0ff40332753 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -1060,7 +1060,7 @@ static int aac_send_reset_adapter(struct aac_dev *dev, void __user *arg)
> return retval;
> }
>
> -int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
> +int aac_do_ioctl(struct aac_dev *dev, unsigned int cmd, void __user *arg)
> {
> int status;
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index e977b459f823..a45f81ec80ce 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -616,7 +616,8 @@ static struct device_attribute *aac_dev_attrs[] = {
> NULL,
> };
>
> -static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> +static int aac_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> @@ -1205,7 +1206,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
> return ret;
> }
>
> -static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> if (!capable(CAP_SYS_RAWIO))
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 8908a20065c8..4d90106fcb37 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -334,7 +334,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t c, res_hndl_t r, u8 mode);
> void cxlflash_list_init(void);
> void cxlflash_term_global_luns(void);
> void cxlflash_free_errpage(void);
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg);
> int cxlflash_mark_contexts_error(struct cxlflash_cfg *cfg);
> void cxlflash_term_local_luns(struct cxlflash_cfg *cfg);
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index c8bad2c093b8..7096810fd222 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -3282,7 +3282,7 @@ static int cxlflash_chr_open(struct inode *inode, struct file *file)
> *
> * Return: A string identifying the decoded host ioctl.
> */
> -static char *decode_hioctl(int cmd)
> +static char *decode_hioctl(unsigned int cmd)
> {
> switch (cmd) {
> case HT_CXLFLASH_LUN_PROVISION:
> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
> index acac6152f50b..1a94a469051e 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -1924,7 +1924,7 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,
> *
> * Return: A string identifying the decoded ioctl.
> */
> -static char *decode_ioctl(int cmd)
> +static char *decode_ioctl(unsigned int cmd)
> {
> switch (cmd) {
> case DK_CXLFLASH_ATTACH:
> @@ -2051,7 +2051,7 @@ static int cxlflash_disk_direct_open(struct scsi_device *sdev, void *arg)
> *
> * Return: 0 on success, -errno on failure
> */
> -static int ioctl_common(struct scsi_device *sdev, int cmd)
> +static int ioctl_common(struct scsi_device *sdev, unsigned int cmd)
> {
> struct cxlflash_cfg *cfg = shost_priv(sdev->host);
> struct device *dev = &cfg->dev->dev;
> @@ -2096,7 +2096,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)
> *
> * Return: 0 on success, -errno on failure
> */
> -int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int cxlflash_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> typedef int (*sioctl) (struct scsi_device *, void *);
>
> @@ -2179,8 +2179,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> }
>
> if (unlikely(copy_from_user(&buf, arg, size))) {
> - dev_err(dev, "%s: copy_from_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_from_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> goto cxlflash_ioctl_exit;
> @@ -2203,8 +2202,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> rc = do_ioctl(sdev, (void *)&buf);
> if (likely(!rc))
> if (unlikely(copy_to_user(arg, &buf, size))) {
> - dev_err(dev, "%s: copy_to_user() fail "
> - "size=%lu cmd=%d (%s) arg=%p\n",
> + dev_err(dev, "%s: copy_to_user() fail size=%lu cmd=%u (%s) arg=%p\n",
> __func__, size, cmd, decode_ioctl(cmd), arg);
> rc = -EFAULT;
> }
> diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
> index 858c3b33db78..7f43b95f4e94 100644
> --- a/drivers/scsi/esas2r/esas2r.h
> +++ b/drivers/scsi/esas2r/esas2r.h
> @@ -965,8 +965,8 @@ struct esas2r_adapter {
> const char *esas2r_info(struct Scsi_Host *);
> int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
> struct esas2r_sas_nvram *data);
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg);
> -int esas2r_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg);
> +int esas2r_ioctl(struct scsi_device *dev, unsigned int cmd, void __user *arg);
> u8 handle_hba_ioctl(struct esas2r_adapter *a,
> struct atto_ioctl *ioctl_hba);
> int esas2r_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd);
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 34bcc8c04ff4..3d130523c288 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -1274,7 +1274,7 @@ int esas2r_write_params(struct esas2r_adapter *a, struct esas2r_request *rq,
>
>
> /* This function only cares about ATTO-specific ioctls (atto_express_ioctl) */
> -int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> +int esas2r_ioctl_handler(void *hostdata, unsigned int cmd, void __user *arg)
> {
> struct atto_express_ioctl *ioctl = NULL;
> struct esas2r_adapter *a;
> @@ -1292,9 +1292,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl = memdup_user(arg, sizeof(struct atto_express_ioctl));
> if (IS_ERR(ioctl)) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler access_ok failed for cmd %d, "
> - "address %p", cmd,
> - arg);
> + "ioctl_handler access_ok failed for cmd %u, address %p",
> + cmd, arg);
> return PTR_ERR(ioctl);
> }
>
> @@ -1493,7 +1492,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> ioctl_done:
>
> if (err < 0) {
> - esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %d", err,
> + esas2r_log(ESAS2R_LOG_WARN, "err %d on ioctl cmd %u", err,
> cmd);
>
> switch (err) {
> @@ -1518,9 +1517,8 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> err = __copy_to_user(arg, ioctl, sizeof(struct atto_express_ioctl));
> if (err != 0) {
> esas2r_log(ESAS2R_LOG_WARN,
> - "ioctl_handler copy_to_user didn't copy "
> - "everything (err %d, cmd %d)", err,
> - cmd);
> + "ioctl_handler copy_to_user didn't copy everything (err %d, cmd %u)",
> + err, cmd);
> kfree(ioctl);
>
> return -EFAULT;
> @@ -1531,7 +1529,7 @@ int esas2r_ioctl_handler(void *hostdata, int cmd, void __user *arg)
> return 0;
> }
>
> -int esas2r_ioctl(struct scsi_device *sd, int cmd, void __user *arg)
> +int esas2r_ioctl(struct scsi_device *sd, unsigned int cmd, void __user *arg)
> {
> return esas2r_ioctl_handler(sd->host->hostdata, cmd, arg);
> }
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index 64397d441bae..fdbda5c05aa0 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -623,7 +623,7 @@ static int esas2r_proc_major;
> long esas2r_proc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
> return esas2r_ioctl_handler(esas2r_proc_host->hostdata,
> - (int)cmd, (void __user *)arg);
> + cmd, (void __user *)arg);
> }
>
> static void __exit esas2r_exit(void)
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 5284444fdd10..f044e7d10d63 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -251,10 +251,11 @@ static int number_of_controllers;
>
> static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
> static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
> #ifdef CONFIG_COMPAT
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> void __user *arg);
> #endif
>
> @@ -6127,7 +6128,7 @@ static void cmd_free(struct ctlr_info *h, struct CommandList *c)
>
> #ifdef CONFIG_COMPAT
>
> -static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> +static int hpsa_ioctl32_passthru(struct scsi_device *dev, unsigned int cmd,
> void __user *arg)
> {
> IOCTL32_Command_struct __user *arg32 =
> @@ -6164,7 +6165,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
> }
>
> static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> - int cmd, void __user *arg)
> + unsigned int cmd, void __user *arg)
> {
> BIG_IOCTL32_Command_struct __user *arg32 =
> (BIG_IOCTL32_Command_struct __user *) arg;
> @@ -6201,7 +6202,8 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
> return err;
> }
>
> -static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> switch (cmd) {
> case CCISS_GETPCIINFO:
> @@ -6521,7 +6523,8 @@ static void check_ioctl_unit_attention(struct ctlr_info *h,
> /*
> * ioctl
> */
> -static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> struct ctlr_info *h;
> void __user *argp = (void __user *)arg;
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index d1b4025a4503..6d053e220153 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -6696,7 +6696,8 @@ static int ipr_queuecommand(struct Scsi_Host *shost,
> * Return value:
> * 0 on success / other on failure
> **/
> -static int ipr_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int ipr_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> struct ipr_resource_entry *res;
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index c43a00a9d819..b775445892af 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -799,7 +799,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost)
> shost->host_failed, tries);
> }
>
> -int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +int sas_ioctl(struct scsi_device *sdev, unsigned int cmd, void __user *arg)
> {
> struct domain_device *dev = sdev_to_domain_dev(sdev);
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index d4aede324e84..3eb47207c7b0 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -832,7 +832,8 @@ static void mk_sense_invalid_opcode(struct scsi_cmnd *scp)
> mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
> }
>
> -static int scsi_debug_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
> +static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg)
> {
> if (sdebug_verbose) {
> if (0x1261 == cmd)
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index f564af8949e8..5d9ccbab7581 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -6043,7 +6043,8 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
> return rc;
> }
>
> -static int pqi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> +static int pqi_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg)
> {
> int rc;
> struct pqi_ctrl_info *ctrl_info;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 68133842e6d7..c9419c05a90a 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1122,10 +1122,11 @@ extern int ata_host_activate(struct ata_host *host, int irq,
> extern void ata_host_detach(struct ata_host *host);
> extern void ata_host_init(struct ata_host *, struct device *, struct ata_port_operations *);
> extern int ata_scsi_detect(struct scsi_host_template *sht);
> -extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
> +extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> extern int ata_scsi_queuecmd(struct Scsi_Host *h, struct scsi_cmnd *cmd);
> extern int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *dev,
> - int cmd, void __user *arg);
> + unsigned int cmd, void __user *arg);
> extern void ata_sas_port_destroy(struct ata_port *);
> extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
> struct ata_port_info *, struct Scsi_Host *);
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 857086cf7ebf..56b2dba7d911 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -707,7 +707,8 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);
>
> extern void sas_target_destroy(struct scsi_target *);
> extern int sas_slave_alloc(struct scsi_device *);
> -extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
> +extern int sas_ioctl(struct scsi_device *sdev, unsigned int cmd,
> + void __user *arg);
> extern int sas_drain_work(struct sas_ha_struct *ha);
>
> extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 6ca954e9f752..4047d68d1b08 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -60,7 +60,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
>
>
> #ifdef CONFIG_COMPAT
> @@ -70,7 +71,8 @@ struct scsi_host_template {
> *
> * Status: OPTIONAL
> */
> - int (* compat_ioctl)(struct scsi_device *dev, int cmd, void __user *arg);
> + int (*compat_ioctl)(struct scsi_device *dev, unsigned int cmd,
> + void __user *arg);
> #endif
>
> /*
> --
> 2.20.1
>


--
Thanks,
~Nick Desaulniers

2019-02-08 08:07:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v5] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Thu, Feb 07, 2019 at 09:07:20AM -0700, Nathan Chancellor wrote:
> Clang warns several times in the scsi subsystem (trimmed for brevity):
>
> drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> case CCISS_GETBUSTYPES:
> ^
> drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> case CCISS_GETHEARTBEAT:
> ^
>
> The root cause is that the _IOC macro can generate really large numbers,
> which don't fit into type 'int', which is used for the cmd parameter in
> the ioctls in scsi_host_template. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful. However, looking at
> the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> cmd parameter, which will fit all of the _IOC values in the scsi/ata
> subsystems.
>
> Make that change because none of the ioctls expect a negative value for
> any command, it brings the ioctls inline with the reset of the kernel,
> and it removes ambiguity, which is never good when dealing with compilers.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/85
> Link: https://github.com/ClangBuiltLinux/linux/issues/154
> Link: https://github.com/ClangBuiltLinux/linux/issues/157
> Signed-off-by: Nathan Chancellor <[email protected]>
> Acked-by: Bradley Grove <[email protected]>
> Acked-by: Don Brace <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2019-02-08 16:01:40

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v5] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template

On Fri, Feb 08, 2019 at 12:05:01AM -0800, Christoph Hellwig wrote:
> On Thu, Feb 07, 2019 at 09:07:20AM -0700, Nathan Chancellor wrote:
> > Clang warns several times in the scsi subsystem (trimmed for brevity):
> >
> > drivers/scsi/hpsa.c:6209:7: warning: overflow converting case value to
> > switch condition type (2147762695 to 18446744071562347015) [-Wswitch]
> > case CCISS_GETBUSTYPES:
> > ^
> > drivers/scsi/hpsa.c:6208:7: warning: overflow converting case value to
> > switch condition type (2147762694 to 18446744071562347014) [-Wswitch]
> > case CCISS_GETHEARTBEAT:
> > ^
> >
> > The root cause is that the _IOC macro can generate really large numbers,
> > which don't fit into type 'int', which is used for the cmd parameter in
> > the ioctls in scsi_host_template. My research into how GCC and Clang are
> > handling this at a low level didn't prove fruitful. However, looking at
> > the rest of the kernel tree, all ioctls use an 'unsigned int' for the
> > cmd parameter, which will fit all of the _IOC values in the scsi/ata
> > subsystems.
> >
> > Make that change because none of the ioctls expect a negative value for
> > any command, it brings the ioctls inline with the reset of the kernel,
> > and it removes ambiguity, which is never good when dealing with compilers.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/85
> > Link: https://github.com/ClangBuiltLinux/linux/issues/154
> > Link: https://github.com/ClangBuiltLinux/linux/issues/157
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > Acked-by: Bradley Grove <[email protected]>
> > Acked-by: Don Brace <[email protected]>
> > Reviewed-by: Bart Van Assche <[email protected]>
> > Tested-by: Nick Desaulniers <[email protected]>
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>

Thank you for the review, it is much appreciated!

Nathan

2019-02-08 22:35:06

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v5] scsi/ata: Use unsigned int for cmd's type in ioctls in scsi_host_template


Nathan,

> Clang warns several times in the scsi subsystem (trimmed for brevity):

Applied to 5.1/scsi-queue, thanks!

--
Martin K. Petersen Oracle Linux Engineering