[PATCH] Off-by-one bug in user page calculations for Direct I/O

On Sun, 16 Nov 2003, Kai Makisara wrote:
> On Sun, 16 Nov 2003, Alan Stern wrote:
>
> > The page count calculations in drivers/scsi/st.c (and copied in sg.c) are
> > wrong.  The code says:
> >
> > 	nr_pages = ((uaddr & ~PAGE_MASK) + count - 1 + ~PAGE_MASK) >>
> > 		PAGE_SHIFT;
> >
> > That will compute an incorrect value if the user's buffer happens to end
> > on the first byte of a new page.  Example:  Suppose uaddr starts right on
>
> Your analysis is correct and this is a bug. Could you send the fix to
> James Bottomley for inclusion into the scsi-bugfixes-2.6 bk tree (at least
> the st part).
>
> Thanks for noticing the bug.
>
> 	Kai
>
> P.S. I usually write these ((base ~ mask) + count + PAGE_SIZE - 1) >>
> PAGE_SHIFT. I don't know why I did it like this here. One should never try
> to be clever and do something in a new way or copy something that does not
> match one's own standard ways of doing things ;-)

On Mon, 17 Nov 2003, Douglas Gilbert wrote:
> Alan,
> ... and the sg part as well ..
>
> > Thanks for noticing the bug.
>
> dito
>
> Doug Gilbert

drivers/scsi/sg.c

@@ -1628,7 +1628,7 @@ st_map_user_pages(struct scatterlist *sgl, const unsigned int max_pages,
 	unsigned int nr_pages;
 	struct page **pages;
 
-	nr_pages = ((uaddr & ~PAGE_MASK) + count - 1 + ~PAGE_MASK) >> PAGE_SHIFT;
+	nr_pages = ((uaddr & ~PAGE_MASK) + count + ~PAGE_MASK) >> PAGE_SHIFT;
 
 	/* User attempted Overflow! */
 	if ((uaddr + count) < uaddr)

drivers/scsi/st.c

@@ -4036,7 +4036,7 @@ static int sgl_map_user_pages(struct scatterlist *sgl, const unsigned int max_pa
 	unsigned int nr_pages;
 	struct page **pages;
 
-	nr_pages = ((uaddr & ~PAGE_MASK) + count - 1 + ~PAGE_MASK) >> PAGE_SHIFT;
+	nr_pages = ((uaddr & ~PAGE_MASK) + count + ~PAGE_MASK) >> PAGE_SHIFT;
 
 	/* User attempted Overflow! */
 	if ((uaddr + count) < uaddr)