[PATCH] st: don't doublefree pages from scatterlist

[Hugh Dickins]

On some architectures, mapping the scatterlist may coalesce entries:
if that coalesced list is then used for freeing the pages afterwards,
there's a danger that pages may be doubly freed (and others leaked).

Fix SCSI Tape's sgl_unmap_user_pages by freeing from the pagelist used
in sgl_map_user_pages.  Fixes Ryan Richter's crash on x86_64, with Bad
page state mapcount 2 from sgl_unmap_user_pages, and consequent mayhem.

But it's quite confusing for st's (un)map_user_pages to be named sgl_,
with sg's named st_: while we're changing its arg, rename st's to
st_map_user_pages and st_unmap_user_pages (and do sg's separately).

Signed-off-by: Hugh Dickins <hugh@xxxxxxxxxxx>
---

 drivers/scsi/st.c |   20 ++++++++++++--------
 drivers/scsi/st.h |    1 +
 2 files changed, 13 insertions(+), 8 deletions(-)

--- 2.6.16-rc2/drivers/scsi/st.c	2006-02-03 09:32:25.000000000 +0000
+++ linux/drivers/scsi/st.c	2006-02-03 09:59:37.000000000 +0000
@@ -188,9 +188,9 @@ static int from_buffer(struct st_buffer 
 static void move_buffer_data(struct st_buffer *, int);
 static void buf_to_sg(struct st_buffer *, unsigned int);
 
-static int sgl_map_user_pages(struct scatterlist *, const unsigned int, 
+static int st_map_user_pages(struct st_buffer *, const unsigned int, 
 			      unsigned long, size_t, int);
-static int sgl_unmap_user_pages(struct scatterlist *, const unsigned int, int);
+static int st_unmap_user_pages(struct st_buffer *, const unsigned int, int);
 
 static int st_probe(struct device *);
 static int st_remove(struct device *);
@@ -1404,7 +1404,7 @@ static int setup_buffering(struct scsi_t
 
 	if (i && ((unsigned long)buf & queue_dma_alignment(
 					STp->device->request_queue)) == 0) {
-		i = sgl_map_user_pages(&(STbp->sg[0]), STbp->use_sg,
+		i = st_map_user_pages(STbp, STbp->use_sg,
 				      (unsigned long)buf, count, (is_read ? READ : WRITE));
 		if (i > 0) {
 			STbp->do_dio = i;
@@ -1456,7 +1456,7 @@ static void release_buffering(struct scs
 
 	STbp = STp->buffer;
 	if (STbp->do_dio) {
-		sgl_unmap_user_pages(&(STbp->sg[0]), STbp->do_dio, is_read);
+		st_unmap_user_pages(STbp, STbp->do_dio, is_read);
 		STbp->do_dio = 0;
 		STbp->sg_segs = 0;
 	}
@@ -4368,13 +4368,14 @@ static void do_create_class_files(struct
 }
 
 /* The following functions may be useful for a larger audience. */
-static int sgl_map_user_pages(struct scatterlist *sgl, const unsigned int max_pages, 
+static int st_map_user_pages(struct st_buffer *STbp, const unsigned int max_pages, 
 			      unsigned long uaddr, size_t count, int rw)
 {
 	unsigned long end = (uaddr + count + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	unsigned long start = uaddr >> PAGE_SHIFT;
 	const int nr_pages = end - start;
 	int res, i, j;
+	struct scatterlist *sgl = STbp->sg;
 	struct page **pages;
 
 	/* User attempted Overflow! */
@@ -4434,7 +4435,7 @@ static int sgl_map_user_pages(struct sca
 		sgl[0].length = count;
 	}
 
-	kfree(pages);
+	STbp->sg_pages = pages;
 	return nr_pages;
 
  out_unmap:
@@ -4449,13 +4450,14 @@ static int sgl_map_user_pages(struct sca
 
 
 /* And unmap them... */
-static int sgl_unmap_user_pages(struct scatterlist *sgl, const unsigned int nr_pages,
+static int st_unmap_user_pages(struct st_buffer *STbp, const unsigned int nr_pages,
 				int dirtied)
 {
 	int i;
 
+	/* Scatterlist entries may have been coalesced: free saved pagelist */
 	for (i=0; i < nr_pages; i++) {
-		struct page *page = sgl[i].page;
+		struct page *page = STbp->sg_pages[i];
 
 		if (dirtied)
 			SetPageDirty(page);
@@ -4465,5 +4467,7 @@ static int sgl_unmap_user_pages(struct s
 		page_cache_release(page);
 	}
 
+	kfree(STbp->sg_pages);
+	STbp->sg_pages = NULL;
 	return 0;
 }
--- 2.6.16-rc2/drivers/scsi/st.h	2006-02-03 09:32:25.000000000 +0000
+++ linux/drivers/scsi/st.h	2006-02-03 09:59:37.000000000 +0000
@@ -49,6 +49,7 @@ struct st_buffer {
 	unsigned short frp_segs;	/* number of buffer segments */
 	unsigned int frp_sg_current;	/* driver buffer length currently in s/g list */
 	struct st_buf_fragment *frp;	/* the allocated buffer fragment list */
+	struct page **sg_pages;		/* pages to be freed from s/g list */
 	struct scatterlist sg[1];	/* MUST BE last item */
 };
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/