On 12/11/2007, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 2007-11-12 at 00:24 +0100, Jesper Juhl wrote: > > From: Jesper Juhl <jesper.juhl@xxxxxxxxx> > > > > 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? > I haven't found any actual problem *use*, I just looked at the size of 'struct smp_resp' and noticed that coverity seemed to be right that 56 bytes are not sufficient to hold the members of the struct. There are 32 bytes in the first members + the union and I don't see how that can ever stay at 56 bytes...? So, we are allocating memory and storing it in a 'struct smp_resp *', but we are allocating less than sizeof(smp_resp) - how is that not a (potential) problem? -- Jesper Juhl <jesper.juhl@xxxxxxxxx> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
