netdev - RE: [PATCH V2 1/6] Drivers: net: hyperv: Enable scatter gather I/O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 8 Mar 2014 04:12:01 +0000
From:	KY Srinivasan <kys@...rosoft.com>
To:	David Miller <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"olaf@...fle.de" <olaf@...fle.de>,
	"apw@...onical.com" <apw@...onical.com>,
	"jasowang@...hat.com" <jasowang@...hat.com>
Subject: RE: [PATCH V2 1/6] Drivers: net: hyperv: Enable scatter gather I/O



> -----Original Message-----
> From: David Miller [mailto:davem@...emloft.net]
> Sent: Saturday, March 8, 2014 3:18 AM
> To: KY Srinivasan
> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> devel@...uxdriverproject.org; olaf@...fle.de; apw@...onical.com;
> jasowang@...hat.com
> Subject: Re: [PATCH V2 1/6] Drivers: net: hyperv: Enable scatter gather I/O
> 
> From: "K. Y. Srinivasan" <kys@...rosoft.com>
> Date: Thu,  6 Mar 2014 21:32:36 -0800
> 
> > +static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
> > +			struct hv_page_buffer *pb)
> > +{
> > +	int j = 0;
> > +
> > +	/* Deal with compund pages by ignoring unused part
> > +	 * of the page.
> > +	 */
> > +	page += offset  >> PAGE_SHIFT;
> 
> Please only one space between "offset" and ">>"
> 
> > +	offset &= ~PAGE_MASK;
> > +
> > +	while (len > 0) {
> > +		unsigned long bytes;
> > +
> > +		bytes = PAGE_SIZE - offset;
> > +		if (bytes > len)
> > +			bytes = len;
> > +		pb[j].pfn = page_to_pfn(page);
> > +		pb[j].offset = offset;
> > +		pb[j].len = bytes;
> > +
> > +		offset += bytes;
> > +		len -= bytes;
> > +
> > +		if (offset == PAGE_SIZE && len) {
> > +			page++;
> > +			offset = 0;
> > +			j++;
> > +		}
> > +	}
> > +
> > +	return j + 1;
> > +}
> 
> I think this function has some edge case errors.
> 
> As I understand it, this function returns how many page buffer entries were
> filled in.
> 
> But if we fill exactly the end of a page, we will report one too many in the
> return value.

I may be missing something here; but in the case you are describing we would return a value
of one as we should. When offset + len is exactly equal to the page size, we would not increment
the value of j, since at that point len == 0. Since the initial value of j was 0 and we return (j + 1),
we will return 1.

Regards,

K. Y
> 
> Consider an "offset" of X, where X is smaller than PAGE_SIZE.  And a "len"
> which is exactly "PAGE_SIZE - offset".
> 
> We will fill in exactly one page buffer entry, however we will erroneously
> return 2 from this function.
> 
> I believe the fix is to first replace the test at the end of the loop
> with:
> 
> 	page++;
> 	offset = 0;
> 	j++;
> 
> And subsequently change the function to just return plain "j".
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html