This function is a huge mess with duplicated error handling. Split out a few useful helpers and use goto labels to untangle the error handling and no-data ioctl handling. Signed-off-by: Christoph Hellwig <hch@xxxxxx> --- drivers/scsi/gdth.c | 250 +++++++++++++++++++++++--------------------- 1 file changed, 132 insertions(+), 118 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 16709735b546..f721af157556 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -4155,131 +4155,145 @@ static int ioc_resetdrv(void __user *arg, char *cmnd) return 0; } -static int ioc_general(void __user *arg, char *cmnd) +static void gdth_ioc_cacheservice(gdth_ha_str *ha, gdth_ioctl_general *gen, + u64 paddr) { - gdth_ioctl_general gen; - char *buf = NULL; - u64 paddr; - gdth_ha_str *ha; - int rval; + if (ha->cache_feat & GDT_64BIT) { + /* copy elements from 32-bit IOCTL structure */ + gen->command.u.cache64.BlockCnt = gen->command.u.cache.BlockCnt; + gen->command.u.cache64.BlockNo = gen->command.u.cache.BlockNo; + gen->command.u.cache64.DeviceNo = gen->command.u.cache.DeviceNo; + + if (ha->cache_feat & SCATTER_GATHER) { + gen->command.u.cache64.DestAddr = (u64)-1; + gen->command.u.cache64.sg_canz = 1; + gen->command.u.cache64.sg_lst[0].sg_ptr = paddr; + gen->command.u.cache64.sg_lst[0].sg_len = gen->data_len; + gen->command.u.cache64.sg_lst[1].sg_len = 0; + } else { + gen->command.u.cache64.DestAddr = paddr; + gen->command.u.cache64.sg_canz = 0; + } + } else { + if (ha->cache_feat & SCATTER_GATHER) { + gen->command.u.cache.DestAddr = 0xffffffff; + gen->command.u.cache.sg_canz = 1; + gen->command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; + gen->command.u.cache.sg_lst[0].sg_len = gen->data_len; + gen->command.u.cache.sg_lst[1].sg_len = 0; + } else { + gen->command.u.cache.DestAddr = paddr; + gen->command.u.cache.sg_canz = 0; + } + } +} - if (copy_from_user(&gen, arg, sizeof(gdth_ioctl_general))) - return -EFAULT; - ha = gdth_find_ha(gen.ionode); - if (!ha) - return -EFAULT; +static void gdth_ioc_scsiraw(gdth_ha_str *ha, gdth_ioctl_general *gen, + u64 paddr) +{ + if (ha->raw_feat & GDT_64BIT) { + /* copy elements from 32-bit IOCTL structure */ + char cmd[16]; + + gen->command.u.raw64.sense_len = gen->command.u.raw.sense_len; + gen->command.u.raw64.bus = gen->command.u.raw.bus; + gen->command.u.raw64.lun = gen->command.u.raw.lun; + gen->command.u.raw64.target = gen->command.u.raw.target; + memcpy(cmd, gen->command.u.raw.cmd, 16); + memcpy(gen->command.u.raw64.cmd, cmd, 16); + gen->command.u.raw64.clen = gen->command.u.raw.clen; + gen->command.u.raw64.sdlen = gen->command.u.raw.sdlen; + gen->command.u.raw64.direction = gen->command.u.raw.direction; + + /* addresses */ + if (ha->raw_feat & SCATTER_GATHER) { + gen->command.u.raw64.sdata = (u64)-1; + gen->command.u.raw64.sg_ranz = 1; + gen->command.u.raw64.sg_lst[0].sg_ptr = paddr; + gen->command.u.raw64.sg_lst[0].sg_len = gen->data_len; + gen->command.u.raw64.sg_lst[1].sg_len = 0; + } else { + gen->command.u.raw64.sdata = paddr; + gen->command.u.raw64.sg_ranz = 0; + } - if (gen.data_len > INT_MAX) - return -EINVAL; - if (gen.sense_len > INT_MAX) - return -EINVAL; - if (gen.data_len + gen.sense_len > INT_MAX) - return -EINVAL; + gen->command.u.raw64.sense_data = paddr + gen->data_len; + } else { + if (ha->raw_feat & SCATTER_GATHER) { + gen->command.u.raw.sdata = 0xffffffff; + gen->command.u.raw.sg_ranz = 1; + gen->command.u.raw.sg_lst[0].sg_ptr = (u32)paddr; + gen->command.u.raw.sg_lst[0].sg_len = gen->data_len; + gen->command.u.raw.sg_lst[1].sg_len = 0; + } else { + gen->command.u.raw.sdata = paddr; + gen->command.u.raw.sg_ranz = 0; + } - if (gen.data_len + gen.sense_len != 0) { - if (!(buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, - FALSE, &paddr))) - return -EFAULT; - if (copy_from_user(buf, arg + sizeof(gdth_ioctl_general), - gen.data_len + gen.sense_len)) { - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return -EFAULT; - } + gen->command.u.raw.sense_data = (u32)paddr + gen->data_len; + } +} - if (gen.command.OpCode == GDT_IOCTL) { - gen.command.u.ioctl.p_param = paddr; - } else if (gen.command.Service == CACHESERVICE) { - if (ha->cache_feat & GDT_64BIT) { - /* copy elements from 32-bit IOCTL structure */ - gen.command.u.cache64.BlockCnt = gen.command.u.cache.BlockCnt; - gen.command.u.cache64.BlockNo = gen.command.u.cache.BlockNo; - gen.command.u.cache64.DeviceNo = gen.command.u.cache.DeviceNo; - /* addresses */ - if (ha->cache_feat & SCATTER_GATHER) { - gen.command.u.cache64.DestAddr = (u64)-1; - gen.command.u.cache64.sg_canz = 1; - gen.command.u.cache64.sg_lst[0].sg_ptr = paddr; - gen.command.u.cache64.sg_lst[0].sg_len = gen.data_len; - gen.command.u.cache64.sg_lst[1].sg_len = 0; - } else { - gen.command.u.cache64.DestAddr = paddr; - gen.command.u.cache64.sg_canz = 0; - } - } else { - if (ha->cache_feat & SCATTER_GATHER) { - gen.command.u.cache.DestAddr = 0xffffffff; - gen.command.u.cache.sg_canz = 1; - gen.command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; - gen.command.u.cache.sg_lst[0].sg_len = gen.data_len; - gen.command.u.cache.sg_lst[1].sg_len = 0; - } else { - gen.command.u.cache.DestAddr = paddr; - gen.command.u.cache.sg_canz = 0; - } - } - } else if (gen.command.Service == SCSIRAWSERVICE) { - if (ha->raw_feat & GDT_64BIT) { - /* copy elements from 32-bit IOCTL structure */ - char cmd[16]; - gen.command.u.raw64.sense_len = gen.command.u.raw.sense_len; - gen.command.u.raw64.bus = gen.command.u.raw.bus; - gen.command.u.raw64.lun = gen.command.u.raw.lun; - gen.command.u.raw64.target = gen.command.u.raw.target; - memcpy(cmd, gen.command.u.raw.cmd, 16); - memcpy(gen.command.u.raw64.cmd, cmd, 16); - gen.command.u.raw64.clen = gen.command.u.raw.clen; - gen.command.u.raw64.sdlen = gen.command.u.raw.sdlen; - gen.command.u.raw64.direction = gen.command.u.raw.direction; - /* addresses */ - if (ha->raw_feat & SCATTER_GATHER) { - gen.command.u.raw64.sdata = (u64)-1; - gen.command.u.raw64.sg_ranz = 1; - gen.command.u.raw64.sg_lst[0].sg_ptr = paddr; - gen.command.u.raw64.sg_lst[0].sg_len = gen.data_len; - gen.command.u.raw64.sg_lst[1].sg_len = 0; - } else { - gen.command.u.raw64.sdata = paddr; - gen.command.u.raw64.sg_ranz = 0; - } - gen.command.u.raw64.sense_data = paddr + gen.data_len; - } else { - if (ha->raw_feat & SCATTER_GATHER) { - gen.command.u.raw.sdata = 0xffffffff; - gen.command.u.raw.sg_ranz = 1; - gen.command.u.raw.sg_lst[0].sg_ptr = (u32)paddr; - gen.command.u.raw.sg_lst[0].sg_len = gen.data_len; - gen.command.u.raw.sg_lst[1].sg_len = 0; - } else { - gen.command.u.raw.sdata = paddr; - gen.command.u.raw.sg_ranz = 0; - } - gen.command.u.raw.sense_data = (u32)paddr + gen.data_len; - } - } else { - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return -EFAULT; - } - } +static int ioc_general(void __user *arg, char *cmnd) +{ + gdth_ioctl_general gen; + gdth_ha_str *ha; + char *buf = NULL; + u64 paddr; + int rval; + + if (copy_from_user(&gen, arg, sizeof(gdth_ioctl_general))) + return -EFAULT; + ha = gdth_find_ha(gen.ionode); + if (!ha) + return -EFAULT; + + if (gen.data_len > INT_MAX) + return -EINVAL; + if (gen.sense_len > INT_MAX) + return -EINVAL; + if (gen.data_len + gen.sense_len > INT_MAX) + return -EINVAL; + + if (gen.data_len + gen.sense_len > 0) { + buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, FALSE, + &paddr); + if (!buf) + return -EFAULT; + + rval = -EFAULT; + if (copy_from_user(buf, arg + sizeof(gdth_ioctl_general), + gen.data_len + gen.sense_len)) + goto out_free_buf; + + if (gen.command.OpCode == GDT_IOCTL) + gen.command.u.ioctl.p_param = paddr; + else if (gen.command.Service == CACHESERVICE) + gdth_ioc_cacheservice(ha, &gen, paddr); + else if (gen.command.Service == SCSIRAWSERVICE) + gdth_ioc_scsiraw(ha, &gen, paddr); + else + goto out_free_buf; + } - rval = __gdth_execute(ha->sdev, &gen.command, cmnd, gen.timeout, &gen.info); - if (rval < 0) { + rval = __gdth_execute(ha->sdev, &gen.command, cmnd, gen.timeout, + &gen.info); + if (rval < 0) + goto out_free_buf; + gen.status = rval; + + rval = -EFAULT; + if (copy_to_user(arg + sizeof(gdth_ioctl_general), buf, + gen.data_len + gen.sense_len)) + goto out_free_buf; + if (copy_to_user(arg, &gen, + sizeof(gdth_ioctl_general) - sizeof(gdth_cmd_str))) + goto out_free_buf; + + rval = 0; +out_free_buf: gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return rval; - } - gen.status = rval; - - if (copy_to_user(arg + sizeof(gdth_ioctl_general), buf, - gen.data_len + gen.sense_len)) { - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return -EFAULT; - } - if (copy_to_user(arg, &gen, - sizeof(gdth_ioctl_general) - sizeof(gdth_cmd_str))) { - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return -EFAULT; - } - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); - return 0; + return rval; } static int ioc_hdrlist(void __user *arg, char *cmnd) -- 2.19.2