linux-kernel - Re: [patch] cache pipe buf page address for non-highmem arch

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]

Message-ID: <b040c32a0703271047vba81938q538d182c9c984f42@mail.gmail.com>
Date:	Tue, 27 Mar 2007 10:47:54 -0700
From:	"Ken Chen" <kenchen@...gle.com>
To:	"Andrew Morton" <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [patch] cache pipe buf page address for non-highmem arch

On 3/26/07, Andrew Morton <akpm@...ux-foundation.org> wrote:
> LTP's vmsplice01 triggers the below:
>
> Unable to handle kernel NULL pointer dereference at 0000000000000130 RIP:
>  [<ffffffff8029e1b6>] pipe_to_file+0x1f3/0x2a6

Ouch, generic_pipe_buf_map() and unmap is used by both pipe and
splice, I better constrain all changes within fs/pipe.c because from
splice path, the page->private is not initialized.

Double ouch for missing test that code path.  Here is an updated
patch, tested with LTP and FIO.  I will write more test cases to make
sure all code path are covered.


diff --git a/fs/pipe.c b/fs/pipe.c
index ebafde7..4b55452 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -21,6 +21,21 @@
 #include <asm/uaccess.h>
 #include <asm/ioctls.h>

+#ifdef CONFIG_HIGHMEM
+#define pipe_kmap		kmap
+#define pipe_kmap_atomic	kmap_atomic
+#define pipe_kunmap		kunmap
+#define pipe_kunmap_atomic	kunmap_atomic
+#else	/* CONFIG_HIGHMEM */
+static inline void *pipe_kmap(struct page *page)
+{
+	return (void *) page->private;
+}
+#define pipe_kmap_atomic(page, type)	pipe_kmap(page)
+#define pipe_kunmap(page)		do { } while (0)
+#define pipe_kunmap_atomic(page, type)	do { } while (0)
+#endif
+
 /*
  * We use a start+len construction, which provides full use of the
  * allocated memory.
@@ -185,6 +200,27 @@ void generic_pipe_buf_unmap
 		kunmap(buf->page);
 }

+static void *pipe_buf_map(struct pipe_inode_info *pipe,
+			   struct pipe_buffer *buf, int atomic)
+{
+	if (atomic) {
+		buf->flags |= PIPE_BUF_FLAG_ATOMIC;
+		return pipe_kmap_atomic(buf->page, KM_USER0);
+	}
+
+	return pipe_kmap(buf->page);
+}
+
+static void pipe_buf_unmap(struct pipe_inode_info *pipe,
+			    struct pipe_buffer *buf, void *map_data)
+{
+	if (buf->flags & PIPE_BUF_FLAG_ATOMIC) {
+		buf->flags &= ~PIPE_BUF_FLAG_ATOMIC;
+		pipe_kunmap_atomic(map_data, KM_USER0);
+	} else
+		pipe_kunmap(buf->page);
+}
+
 int generic_pipe_buf_steal(struct pipe_inode_info *pipe,
 			   struct pipe_buffer *buf)
 {
@@ -210,8 +246,8 @@ int generic_pipe_buf_pin

 static const struct pipe_buf_operations anon_pipe_buf_ops = {
 	.can_merge = 1,
-	.map = generic_pipe_buf_map,
-	.unmap = generic_pipe_buf_unmap,
+	.map = pipe_buf_map,
+	.unmap = pipe_buf_unmap,
 	.pin = generic_pipe_buf_pin,
 	.release = anon_pipe_buf_release,
 	.steal = generic_pipe_buf_steal,
@@ -316,6 +352,7 @@ redo:
 		if (do_wakeup) {
 			wake_up_interruptible_sync(&pipe->wait);
  			kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
+			do_wakeup = 0;
 		}
 		pipe_wait(pipe);
 	}
@@ -423,6 +460,8 @@ redo1:
 					ret = ret ? : -ENOMEM;
 					break;
 				}
+				page->private = (unsigned long)
+							page_address(page);
 				pipe->tmp_page = page;
 			}
 			/* Always wake up, even if the copy fails. Otherwise
@@ -438,16 +477,16 @@ redo1:
 			iov_fault_in_pages_read(iov, chars);
 redo2:
 			if (atomic)
-				src = kmap_atomic(page, KM_USER0);
+				src = pipe_kmap_atomic(page, KM_USER0);
 			else
-				src = kmap(page);
+				src = pipe_kmap(page);

 			error = pipe_iov_copy_from_user(src, iov, chars,
 							atomic);
 			if (atomic)
-				kunmap_atomic(src, KM_USER0);
+				pipe_kunmap_atomic(src, KM_USER0);
 			else
-				kunmap(page);
+				pipe_kunmap(page);

 			if (unlikely(error)) {
 				if (atomic) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/