On Fri, 4 Jul 2014, Julia Lawall wrote: > > > On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote: > > > > > > > > -----Original Message----- > > > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Himangi Saraogi > > > Sent: Friday, 04 July, 2014 1:28 PM > > > To: Vikas Chaudhary; iscsi-driver@xxxxxxxxxx; James E.J. Bottomley; linux- > > > scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > > > Cc: julia.lawall@xxxxxxx > > > Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure > > > > > > In this code, 0 is returned on memory allocation failure, even though > > > other failures return -ENOMEM or other similar values. > > > > > > A simplified version of the Coccinelle semantic match that finds this > > > problem is as follows: > > > > > > // <smpl> > > > @@ > > > expression ret; > > > expression x,e1,e2,e3; > > > identifier alloc; > > > @@ > > > > > > ret = 0 > > > ... when != ret = e1 > > > *x = alloc(...) > > > ... when != ret = e2 > > > if (x == NULL) { ... when != ret = e3 > > > return ret; > > > } > > > // </smpl> > > > > > > Signed-off-by: Himangi Saraogi <himangi774@xxxxxxxxx> > > > Acked-by: Julia Lawall <julia.lawall@xxxxxxx> > > > --- > > > drivers/scsi/qla4xxx/ql4_os.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > > > index c5d9564..72ba671 100644 > > > --- a/drivers/scsi/qla4xxx/ql4_os.c > > > +++ b/drivers/scsi/qla4xxx/ql4_os.c > > > @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host > > > *shost, char *buf, int len) > > > if (!ql_iscsi_stats) { > > > ql4_printk(KERN_ERR, ha, > > > "Unable to allocate memory for iscsi stats\n"); > > > + ret = -ENOMEM; > > > goto exit_host_stats; > > > } > > > > > > > Also, the only caller of this function doesn't use the return > > value - it's overwritten by another function call: > > > > drivers/scsi/scsi_transport_iscsi.c: > > err = transport->get_host_stats(shost, buf, host_stats_size); > > > > actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size); > > skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size)); > > nlhhost_stats->nlmsg_len = actual_size; > > > > err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID, > > GFP_KERNEL); > > Maybe that is intentional? If get_host_stats fails, eg because of a lack > of memory, the worst that will happen is that a region of the buffer will > be 0. If the loop is done again, there seems to be a risk of an infinite > loop. Or one could jump out of the function completely, as on failure of alloc_skb. julia -- 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
