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. 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
