[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

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index debf89dff08e3a2f7cf74e09c97d6792e5f9603b..d4b4330c2887deaf3a20c87801e29da7d8c948a2 100644 (file)
--- a/drivers/scsi/sg.c
+++ b/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)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index ed0a5ab2667b95b871a88ed25b057eeb0827c635..a64e43ac2601ab461d0a30e5bf3c6f99df7047a5 100644 (file)
--- a/drivers/scsi/st.c
+++ b/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)