Hi Dan, > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Thursday, August 20, 2015 4:01 PM > To: Amitkumar Karwar; Bing Zhao > Cc: Nishant Sarmukadam; Kalle Valo; linux-wireless@xxxxxxxxxxxxxxx; > kernel-janitors@xxxxxxxxxxxxxxx > Subject: [patch] mwifiex: fix mwifiex_rdeeprom_read() > > There were several bugs here. > > 1) The done label was in the wrong place so we didn't copy any > information out when there was no command given. > > 2) We were using PAGE_SIZE as the size of the buffer instead of > "PAGE_SIZE - pos". > > 3) snprintf() returns the number of characters that would have been > printed if there were enough space. If there was not enough space > (and we had fixed the memory corruption bug #2) then it would result > in an information leak when we do simple_read_from_buffer(). I've > changed it to use scnprintf() instead. > > I also removed the initialization at the start of the function, because > I thought it made the code a little more clear. > > Fixes: 5e6e3a92b9a4 ('wireless: mwifiex: initial commit for Marvell > mwifiex driver') > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > diff --git a/drivers/net/wireless/mwifiex/debugfs.c > b/drivers/net/wireless/mwifiex/debugfs.c > index 5a0636d4..bd2f5d2 100644 > --- a/drivers/net/wireless/mwifiex/debugfs.c > +++ b/drivers/net/wireless/mwifiex/debugfs.c > @@ -731,7 +731,7 @@ mwifiex_rdeeprom_read(struct file *file, char __user > *ubuf, > (struct mwifiex_private *) file->private_data; > unsigned long addr = get_zeroed_page(GFP_KERNEL); > char *buf = (char *) addr; > - int pos = 0, ret = 0, i; > + int pos, ret, i; > u8 value[MAX_EEPROM_DATA]; > > if (!buf) > @@ -739,7 +739,7 @@ mwifiex_rdeeprom_read(struct file *file, char __user > *ubuf, > > if (saved_offset == -1) { > /* No command has been given */ > - pos += snprintf(buf, PAGE_SIZE, "0"); > + pos = snprintf(buf, PAGE_SIZE, "0"); > goto done; > } > > @@ -751,14 +751,14 @@ mwifiex_rdeeprom_read(struct file *file, char > __user *ubuf, > goto done; > } > > - pos += snprintf(buf, PAGE_SIZE, "%d %d ", saved_offset, > saved_bytes); > + pos = snprintf(buf, PAGE_SIZE, "%d %d ", saved_offset, > saved_bytes); > > for (i = 0; i < saved_bytes; i++) > - pos += snprintf(buf + strlen(buf), PAGE_SIZE, "%d ", > value[i]); > + pos += scnprintf(buf + pos, PAGE_SIZE - pos, "%d ", > value[i]); > > +done: > ret = simple_read_from_buffer(ubuf, count, ppos, buf, pos); > > -done: > free_page(addr); > return ret; > } Looks good. Acked-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> Regards, Amitkumar -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
