re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove,
The patch 26780d9e12ed: "[SCSI] esas2r: ATTO Technology ExpressSAS 6G
SAS/SATA RAID Adapter Driver" from Aug 23, 2013, leads to the
following static checker warning:
drivers/scsi/esas2r/esas2r_ioctl.c:1902 esas2r_read_vda()
error: 'count' from user is not capped properly
drivers/scsi/esas2r/esas2r_ioctl.c
1892
1893 if (off > VDA_MAX_BUFFER_SIZE)
1894 return 0;
1895
1896 if (count + off > VDA_MAX_BUFFER_SIZE)
^
"count" is a user controlled int. Let's assume we're on a 32 system for
simplicity. If count is a high positive number here, then count + off
is negative and thus less than VDA_MAX_BUFFER_SIZE.
1897 count = VDA_MAX_BUFFER_SIZE - off;
1898
1899 if (count < 0)
1900 return 0;
1901
1902 memcpy(buf, a->vda_buffer + off, count);
^^^
Memory corruption.
1903
1904 return count;
1905 }
"count" comes from the ioctl. Let's look at that:
drivers/scsi/esas2r/esas2r_ioctl.c
1476 case EXPRESS_IOCTL_VDA:
1477 err = esas2r_write_vda(a,
1478 (char *)&ioctl->data.ioctl_vda,
1479 0,
1480 sizeof(struct atto_ioctl_vda) +
1481
ioctl->data.ioctl_vda.data_length);
^^
1482
1483 if (err >= 0) {
1484 err = esas2r_read_vda(a,
1485(char
*)&ioctl->data.ioctl_vda,
14860,
1487sizeof(struct
atto_ioctl_vda) +
1488
ioctl->data.ioctl_vda.data_length);
^
These additions have integer overflow bugs. It seems harmless to me,
but hopefully static checkers will eventually start complaining about
them.
1489 }
1490
1491
1492
1493
1494 break;
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
> +struct esas2r_adapter {
> +struct esas2r_target targetdb[ESAS2R_MAX_TARGETS];
> +struct esas2r_target *targetdb_end;
...
> +u8 fw_coredump_buff[ESAS2R_FWCOREDUMP_SZ];
> +void esas2r_reset_chip(struct esas2r_adapter *a)
> +{
> +if (!esas2r_is_adapter_present(a))
> +return;
> +
> +/*
> + * Before we reset the chip, save off the VDA core dump. The VDA core
> + * dump is located in the upper 512KB of the onchip SRAM. Make sure
> + * to not overwrite a previous crash that was saved.
> + */
> +if ((a->flags2 & AF2_COREDUMP_AVAIL)
> +&& !(a->flags2 & AF2_COREDUMP_SAVED)
> +&& a->fw_coredump_buff) {
> +esas2r_read_mem_block(a,
> + a->fw_coredump_buff,
> + MW_DATA_ADDR_SRAM + 0x8,
> + ESAS2R_FWCOREDUMP_SZ);
Comparing an array (fw_coredump_buff) to null probably isn't what you intended
here.
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove, The patch 17adeb6dabbe: "[SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver" from Aug 23, 2013, leads to the following Smatch warning: "drivers/scsi/esas2r/esas2r_vda.c:312 esas2r_complete_vda_ioctl() error: format string overflow. buf_size: 4 length: 5" drivers/scsi/esas2r/esas2r_vda.c 312 sprintf((char *)&cfg->data.init.fw_release, ^ This is a u32 but we are writing 4 characters and a NUL so it ends up putting the NUL in cfg->data.init.epoch_time. 313 "%1d.%02d", 314 (int)LOBYTE(le16_to_cpu(rsp->fw_release)), 315 (int)HIBYTE(le16_to_cpu(rsp->fw_release))); regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove,
This is a semi-automatic email about new static checker warnings.
The patch 17adeb6dabbe: "[SCSI] esas2r: ATTO Technology ExpressSAS 6G
SAS/SATA RAID Adapter Driver" from Aug 23, 2013, leads to the
following Smatch complaint:
drivers/scsi/esas2r/esas2r_init.c:671 esas2r_cleanup()
warn: variable dereferenced before check 'host' (see line 668)
drivers/scsi/esas2r/esas2r_init.c
667 {
668 struct esas2r_adapter *a = (struct esas2r_adapter
*)host->hostdata;
^^
Patch adds dereference.
669 int index;
670
671 if (host == NULL) {
Patch adds check.
672 int i;
673
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/10] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
On Fri, 2013-08-23 at 10:35 -0400, Bradley Grove wrote: > This is a new driver for ATTO Technology's ExpressSAS series of > hardware RAID > adapters. It supports the following adapters: > > - ExpressSAS R60F > - ExpressSAS R680 > - ExpressSAS R608 > - ExpressSAS R644 OK, I put this in misc. I suspect I was the only person who actually reviewed it, so I want to see what the static checkers report on it, but if they don't spit back anything too bad, I think it can go into the next merge window. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/10] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
This is a new driver for ATTO Technology's ExpressSAS series of hardware RAID adapters. It supports the following adapters: - ExpressSAS R60F - ExpressSAS R680 - ExpressSAS R608 - ExpressSAS R644 This patch is split into ten parts, all of which need to be applied to build the driver. Changes Since V3: - Addressed some warnings reported by checkpatch script, mostly whitespace Changes since V2: - Fix spurious ioctl error logging and memory corruption occuring with non-IO VDA requests - Use kernel's schedule_timeout_interruptable() rather than our own own stall function - Removed a UDP socket based interface used by our closed source configuration tool Changes since V1: - We now use the kernel's list structs and alignment macros rather than our own implementations - Fix to PCIe address size declaration when compiling on 32-bit architectures - Use memcpy() instead opf memmove() where appropriate - Fixes for big-endian architectures - Removed unneeded MSI configuration code - Simplified memory allocation during init - Code formatting cleanup Bradley Grove (10): SCSI: esas2r: Add main header file SCSI: esas2r: Add main file SCSI: esas2r: Add initialization functions SCSI: esas2r: Add device discovery and logging functions SCSI: esas2r: Add interrupt and IO functions SCSI: esas2r: Add flash and target database functions SCSI: esas2r: Add IOCTL header file SCSI: esas2r: Add IOCTL functions SCSI: esas2r: Add ATTO VDA Firmware API headers and functions. This API is used to control and manage the RAID adapter. SCSI: esas2r: Add Makefile, Kconfig, and MAINTAINERS files MAINTAINERS |7 + drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/esas2r/Kconfig |6 + drivers/scsi/esas2r/Makefile|5 + drivers/scsi/esas2r/atioctl.h | 1254 + drivers/scsi/esas2r/atvda.h | 1320 ++ drivers/scsi/esas2r/esas2r.h| 1441 drivers/scsi/esas2r/esas2r_disc.c | 1190 drivers/scsi/esas2r/esas2r_flash.c | 1514 + drivers/scsi/esas2r/esas2r_init.c | 1773 + drivers/scsi/esas2r/esas2r_int.c| 941 drivers/scsi/esas2r/esas2r_io.c | 883 +++ drivers/scsi/esas2r/esas2r_ioctl.c | 2110 +++ drivers/scsi/esas2r/esas2r_log.c| 255 + drivers/scsi/esas2r/esas2r_log.h| 118 ++ drivers/scsi/esas2r/esas2r_main.c | 2032 + drivers/scsi/esas2r/esas2r_targdb.c | 306 + drivers/scsi/esas2r/esas2r_vda.c| 523 + 19 files changed, 15680 insertions(+) create mode 100644 drivers/scsi/esas2r/Kconfig create mode 100644 drivers/scsi/esas2r/Makefile create mode 100644 drivers/scsi/esas2r/atioctl.h create mode 100644 drivers/scsi/esas2r/atvda.h create mode 100644 drivers/scsi/esas2r/esas2r.h create mode 100644 drivers/scsi/esas2r/esas2r_disc.c create mode 100644 drivers/scsi/esas2r/esas2r_flash.c create mode 100644 drivers/scsi/esas2r/esas2r_init.c create mode 100644 drivers/scsi/esas2r/esas2r_int.c create mode 100644 drivers/scsi/esas2r/esas2r_io.c create mode 100644 drivers/scsi/esas2r/esas2r_ioctl.c create mode 100644 drivers/scsi/esas2r/esas2r_log.c create mode 100644 drivers/scsi/esas2r/esas2r_log.h create mode 100644 drivers/scsi/esas2r/esas2r_main.c create mode 100644 drivers/scsi/esas2r/esas2r_targdb.c create mode 100644 drivers/scsi/esas2r/esas2r_vda.c -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/10] [RFC] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
This is a new driver for ATTO Technology's ExpressSAS series of hardware RAID adapters. It supports the following adapters: - ExpressSAS R60F - ExpressSAS R680 - ExpressSAS R608 - ExpressSAS R644 This patch is split into ten parts, all of which need to be applied to build the driver. Changes since V2: - Fix spurious ioctl error logging and memory corruption occuring with non-IO VDA requests - Use kernel's schedule_timeout_interruptable() rather than our own own stall function - Removed a UDP socket based interface used by our closed source configuration tool Changes since V1: - We now use the kernel's list structs and alignment macros rather than our own implementations - Fix to PCIe address size declaration when compiling on 32-bit architectures - Use memcpy() instead opf memmove() where appropriate - Fixes for big-endian architectures - Removed unneeded MSI configuration code - Simplified memory allocation during init - Code formatting cleanup Bradley Grove (10): [RFC] SCSI: esas2r: Add main header file [RFC] SCSI: esas2r: Add main file [RFC] SCSI: esas2r: Add initialization functions [RFC] SCSI: esas2r: Add device discovery and logging functions [RFC] SCSI: esas2r: Add interrupt and IO functions [RFC] SCSI: esas2r: Add flash and target database functions [RFC] SCSI: esas2r: Add IOCTL header file [RFC] SCSI: esas2r: Add IOCTL functions [RFC] SCSI: esas2r: Add ATTO VDA Firmware API headers and functions. This API is used to control and manage the RAID adapter. [RFC] SCSI: esas2r: Add Makefile, Kconfig, and MAINTAINERS files MAINTAINERS |7 + drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/esas2r/Kconfig |6 + drivers/scsi/esas2r/Makefile|5 + drivers/scsi/esas2r/atioctl.h | 1254 + drivers/scsi/esas2r/atvda.h | 1320 ++ drivers/scsi/esas2r/esas2r.h| 1441 drivers/scsi/esas2r/esas2r_disc.c | 1190 drivers/scsi/esas2r/esas2r_flash.c | 1514 + drivers/scsi/esas2r/esas2r_init.c | 1773 + drivers/scsi/esas2r/esas2r_int.c| 941 drivers/scsi/esas2r/esas2r_io.c | 883 +++ drivers/scsi/esas2r/esas2r_ioctl.c | 2110 +++ drivers/scsi/esas2r/esas2r_log.c| 255 + drivers/scsi/esas2r/esas2r_log.h| 118 ++ drivers/scsi/esas2r/esas2r_main.c | 2033 + drivers/scsi/esas2r/esas2r_targdb.c | 306 + drivers/scsi/esas2r/esas2r_vda.c| 523 + 19 files changed, 15681 insertions(+) create mode 100644 drivers/scsi/esas2r/Kconfig create mode 100644 drivers/scsi/esas2r/Makefile create mode 100644 drivers/scsi/esas2r/atioctl.h create mode 100644 drivers/scsi/esas2r/atvda.h create mode 100644 drivers/scsi/esas2r/esas2r.h create mode 100644 drivers/scsi/esas2r/esas2r_disc.c create mode 100644 drivers/scsi/esas2r/esas2r_flash.c create mode 100644 drivers/scsi/esas2r/esas2r_init.c create mode 100644 drivers/scsi/esas2r/esas2r_int.c create mode 100644 drivers/scsi/esas2r/esas2r_io.c create mode 100644 drivers/scsi/esas2r/esas2r_ioctl.c create mode 100644 drivers/scsi/esas2r/esas2r_log.c create mode 100644 drivers/scsi/esas2r/esas2r_log.h create mode 100644 drivers/scsi/esas2r/esas2r_main.c create mode 100644 drivers/scsi/esas2r/esas2r_targdb.c create mode 100644 drivers/scsi/esas2r/esas2r_vda.c -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/10] [RFC] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
This is a new driver for ATTO Technology's ExpressSAS series of hardware RAID adapters. It supports the following adapters: - ExpressSAS R60F - ExpressSAS R680 - ExpressSAS R608 - ExpressSAS R644 This patch is split into ten parts, all of which need to be applied to build the driver. Changes since V1: - We now use the kernel's list structs and alignment macros rather than our own implementations - Fix to PCIe address size declaration when compiling on 32-bit architectures - Use memcpy() instead opf memmove() where appropriate - Fixes for big-endian architectures - Removed unneeded MSI configuration code - Simplified memory allocation during init - Code formatting cleanup Bradley Grove (10): [RFC] SCSI: esas2r: Add main header file [RFC] SCSI: esas2r: Add main file [RFC] SCSI: esas2r: Add initialization functions [RFC] SCSI: esas2r: Add device discovery and logging functions [RFC] SCSI: esas2r: Add interrupt and IO functions [RFC] SCSI: esas2r: Add flash and target database functions [RFC] SCSI: esas2r: Add IOCTL header file [RFC] SCSI: esas2r: Add IOCTL functions [RFC] SCSI: esas2r: Add ATTO VDA Firmware API headers and functions. This API is used to control and manage the RAID adapter. [RFC] SCSI: esas2r: Add Makefile, Kconfig, and MAINTAINERS files MAINTAINERS |7 + drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/esas2r/Kconfig |6 + drivers/scsi/esas2r/Makefile|5 + drivers/scsi/esas2r/atioctl.h | 1254 drivers/scsi/esas2r/atvda.h | 1320 + drivers/scsi/esas2r/esas2r.h| 1442 +++ drivers/scsi/esas2r/esas2r_disc.c | 1190 +++ drivers/scsi/esas2r/esas2r_flash.c | 1516 drivers/scsi/esas2r/esas2r_init.c | 1775 drivers/scsi/esas2r/esas2r_int.c| 940 +++ drivers/scsi/esas2r/esas2r_io.c | 883 ++ drivers/scsi/esas2r/esas2r_ioctl.c | 2108 ++ drivers/scsi/esas2r/esas2r_log.c| 255 drivers/scsi/esas2r/esas2r_log.h| 118 ++ drivers/scsi/esas2r/esas2r_main.c | 2169 +++ drivers/scsi/esas2r/esas2r_targdb.c | 306 + drivers/scsi/esas2r/esas2r_vda.c| 523 + 19 files changed, 15819 insertions(+) create mode 100644 drivers/scsi/esas2r/Kconfig create mode 100644 drivers/scsi/esas2r/Makefile create mode 100644 drivers/scsi/esas2r/atioctl.h create mode 100644 drivers/scsi/esas2r/atvda.h create mode 100644 drivers/scsi/esas2r/esas2r.h create mode 100644 drivers/scsi/esas2r/esas2r_disc.c create mode 100644 drivers/scsi/esas2r/esas2r_flash.c create mode 100644 drivers/scsi/esas2r/esas2r_init.c create mode 100644 drivers/scsi/esas2r/esas2r_int.c create mode 100644 drivers/scsi/esas2r/esas2r_io.c create mode 100644 drivers/scsi/esas2r/esas2r_ioctl.c create mode 100644 drivers/scsi/esas2r/esas2r_log.c create mode 100644 drivers/scsi/esas2r/esas2r_log.h create mode 100644 drivers/scsi/esas2r/esas2r_main.c create mode 100644 drivers/scsi/esas2r/esas2r_targdb.c create mode 100644 drivers/scsi/esas2r/esas2r_vda.c -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/10] [RFC] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
On Thu, 2013-06-13 at 10:30 -0400, Bradley Grove wrote: > This is a new driver for ATTO Technology's ExpressSAS series of hardware RAID > adapters. It supports the following adapters: > > - ExpressSAS R60F > - ExpressSAS R680 > - ExpressSAS R608 > - ExpressSAS R644 > > This patch is split into ten parts to make reviewing easier. You'll need to > apply all of them to get the complete patch. > > We have done some testing under x86 and x64 architectures, but the code is > still under development and we expect to find additional issues. We > appreciate > any review comments. This isn't an exhaustive review by any means, but what I noticed just from a quick skim over the driver is this: > +#ifdef HOST_BIG_ENDIAN > + *((u16 *)&vrq->scsi.handle + 0) = a->cmd_ref_no++; > +#else > + *((u16 *)&vrq->scsi.handle + 1) = a->cmd_ref_no++; > +#endif Looks obviously wrong (there is no HOST_BIG_ENDIAN define in the code). I think what you want here is *((u16 *)&vrq->scsi.handle) = cpu_to_be16(a->cmd_ref_no++); There's also a lot of open coded linked list handling code like this: > +/* Remove a request from a double-linked list */ > +static inline void esas2r_dequeue(struct esas2r_request *rq) and > +/* Add a request to a double-linked list */ > +static inline void esas2r_enqueue(struct esas2r_request *head, > + struct esas2r_request *rq) Please don't do this. Use the linux struct list_head (in list.h) instead ... it's a common pattern that everyone thinks they have to write their own linked lists and everyone invariably makes a mistake doing it ... just using the provided handlers avoids a lot of the problems. You also have some hidden singly linked list open coding (like the free_sg_list) which you should use the struct hlist primitives for. Could you go through the driver and check for primitives which should already be in Linux? Another example of this is all the alignment ones you have in the file ... you should be using the ALIGN macro. You have another pattern of using memmove() where you should be using memcpy(). memmove() is for the case where source and destination may overlap and it can be less efficient than memcpy on a lot of platforms. If I read the code right, you've got a lun limit at 255, so you should probably be setting max_lun in the host template (and checking the value) to prevent accidents since you use the rest of the flags word for command data. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/10] [RFC] SCSI: esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
This is a new driver for ATTO Technology's ExpressSAS series of hardware RAID adapters. It supports the following adapters: - ExpressSAS R60F - ExpressSAS R680 - ExpressSAS R608 - ExpressSAS R644 This patch is split into ten parts to make reviewing easier. You'll need to apply all of them to get the complete patch. We have done some testing under x86 and x64 architectures, but the code is still under development and we expect to find additional issues. We appreciate any review comments. Bradley Grove (10): [RFC] SCSI: esas2r: Add main header file [RFC] SCSI: esas2r: Add main file [RFC] SCSI: esas2r: Add initialization functions [RFC] SCSI: esas2r: Add device discovery and logging functions [RFC] SCSI: esas2r: Add interrupt and IO functions [RFC] SCSI: esas2r: Add flash and target database functions [RFC] SCSI: esas2r: Add IOCTL header file [RFC] SCSI: esas2r: Add IOCTL functions [RFC] SCSI: esas2r: Add ATTO VDA Firmware API headers and functions. This API is used to control and manage the RAID adapter. [RFC] SCSI: esas2r: Add Makefile, Kconfig, and MAINTAINERS files MAINTAINERS |9 +- drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/esas2r/Kconfig |6 + drivers/scsi/esas2r/Makefile|6 + drivers/scsi/esas2r/atioctl.h | 1254 drivers/scsi/esas2r/atvda.h | 1327 + drivers/scsi/esas2r/esas2r.h| 1755 drivers/scsi/esas2r/esas2r_disc.c | 1192 +++ drivers/scsi/esas2r/esas2r_flash.c | 1518 drivers/scsi/esas2r/esas2r_init.c | 2019 drivers/scsi/esas2r/esas2r_int.c| 935 +++ drivers/scsi/esas2r/esas2r_io.c | 886 ++ drivers/scsi/esas2r/esas2r_ioctl.c | 2110 + drivers/scsi/esas2r/esas2r_log.c| 255 drivers/scsi/esas2r/esas2r_log.h| 118 ++ drivers/scsi/esas2r/esas2r_main.c | 2177 +++ drivers/scsi/esas2r/esas2r_targdb.c | 306 + drivers/scsi/esas2r/esas2r_vda.c| 526 + 19 files changed, 16400 insertions(+), 1 deletion(-) create mode 100644 drivers/scsi/esas2r/Kconfig create mode 100644 drivers/scsi/esas2r/Makefile create mode 100644 drivers/scsi/esas2r/atioctl.h create mode 100644 drivers/scsi/esas2r/atvda.h create mode 100644 drivers/scsi/esas2r/esas2r.h create mode 100644 drivers/scsi/esas2r/esas2r_disc.c create mode 100644 drivers/scsi/esas2r/esas2r_flash.c create mode 100644 drivers/scsi/esas2r/esas2r_init.c create mode 100644 drivers/scsi/esas2r/esas2r_int.c create mode 100644 drivers/scsi/esas2r/esas2r_io.c create mode 100644 drivers/scsi/esas2r/esas2r_ioctl.c create mode 100644 drivers/scsi/esas2r/esas2r_log.c create mode 100644 drivers/scsi/esas2r/esas2r_log.h create mode 100644 drivers/scsi/esas2r/esas2r_main.c create mode 100644 drivers/scsi/esas2r/esas2r_targdb.c create mode 100644 drivers/scsi/esas2r/esas2r_vda.c -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
