Messages in this thread | | | Date | Sun, 11 Nov 2007 19:33:43 -0500 | From | Douglas Gilbert <> | Subject | Re: [PATCH] Fix problem with size of allocation in libsas |
| |
James Bottomley wrote: > On Mon, 2007-11-12 at 00:24 +0100, Jesper Juhl wrote: >> From: Jesper Juhl <jesper.juhl@gmail.com> >> >> in sas_get_phy_change_count(), the line >> disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); >> will allocate 56 bytes due to this define: >> #define DISCOVER_RESP_SIZE 56 >> But, the struct is actually 60 bytes in size. >> >> So change the define to be >> #define DISCOVER_RESP_SIZE sizeof(struct smp_resp) >> so we always get the correct size even when people >> fiddle with the structure. >> >> This change also fixes the same problem in >> sas_get_phy_attached_sas_addr() >> >> (Found by the Coverity checker. Compile tested only) > > Well, your fix is definitely wrong. > > Could you explain the problem a little more? The discover response SMP > frame is 56 bytes as mandated by the standard. I don't see anywhere in > the code where we're actually using a value beyond the 56th byte ... > where is the problem use?
The response size of SMP DISCOVER keeps growing with each rev. Currently in SAS-2 revision 12 it is 112 bytes long! The original SAS standard and SAS 1.1 have implicit response sizes and for DISCOVER that is 56 bytes.
To be compliant with SAS-2 the code should read byte 3 of a DISCOVER response. If it is zero then the response length is 56 bytes, otherwise the response length is that many dwords (i.e. 4 byte units) plus 4 bytes for the CRC. [Similar logic applies to many other SMP responses.]
I have tables for all SMP requests and response (up until SAS-2 rev 12) in my smp_utils package, see the smp_lib.c file.
Doug Gilbert - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |