linux-kernel - [PATCH] pipe: avoid creating empty pipe buffers

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>] [day] [month] [year] [list]

Message-ID: <20221213110212.649796-1-wiktorg@google.com>
Date:   Tue, 13 Dec 2022 12:02:12 +0100
From:   Wiktor Garbacz <wiktorg@...gle.com>
To:     Alexander Viro <viro@...iv.linux.org.uk>,
        David Howells <dhowells@...hat.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wiktor Garbacz <wiktorg@...gle.com>, stable@...r.kernel.org
Subject: [PATCH] pipe: avoid creating empty pipe buffers

pipe_write cannot be called on notification pipes so
post_one_notification cannot race it.
Locking and second pipe_full check are thus redundant.

This fixes an issue where pipe write could unexpectedly block:
  // Assume there is no reader or reader polls and uses FIONREAD ioctl
  // to read all the available bytes.
  for (int i = 0; i < PIPE_DEF_BUFFERS+1; ++i) {
    write(pipe_fd, buf_that_efaults, PAGE_SIZE);
  }
  // Never reached

Fixes: a194dfe6e6f6 ("pipe: Rearrange sequence in pipe_write() to preallocate slot")
Cc: stable@...r.kernel.org
Signed-off-by: Wiktor Garbacz <wiktorg@...gle.com>
---
 fs/pipe.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 42c7ff41c2db..87356a2823cf 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -501,43 +501,26 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 				pipe->tmp_page = page;
 			}
 
-			/* Allocate a slot in the ring in advance and attach an
-			 * empty buffer.  If we fault or otherwise fail to use
-			 * it, either the reader will consume it or it'll still
-			 * be there for the next write.
-			 */
-			spin_lock_irq(&pipe->rd_wait.lock);
-
-			head = pipe->head;
-			if (pipe_full(head, pipe->tail, pipe->max_usage)) {
-				spin_unlock_irq(&pipe->rd_wait.lock);
-				continue;
+			copied = copy_page_from_iter(page, 0, PAGE_SIZE, from);
+			if (unlikely(copied < PAGE_SIZE && iov_iter_count(from))) {
+				if (!ret)
+					ret = -EFAULT;
+				break;
 			}
-
-			pipe->head = head + 1;
-			spin_unlock_irq(&pipe->rd_wait.lock);
+			ret += copied;
 
 			/* Insert it into the buffer array */
-			buf = &pipe->bufs[head & mask];
 			buf->page = page;
 			buf->ops = &anon_pipe_buf_ops;
 			buf->offset = 0;
-			buf->len = 0;
+			buf->len = copied;
 			if (is_packetized(filp))
 				buf->flags = PIPE_BUF_FLAG_PACKET;
 			else
 				buf->flags = PIPE_BUF_FLAG_CAN_MERGE;
 			pipe->tmp_page = NULL;
-
-			copied = copy_page_from_iter(page, 0, PAGE_SIZE, from);
-			if (unlikely(copied < PAGE_SIZE && iov_iter_count(from))) {
-				if (!ret)
-					ret = -EFAULT;
-				break;
-			}
-			ret += copied;
-			buf->offset = 0;
-			buf->len = copied;
+			head++;
+			pipe->head = head;
 
 			if (!iov_iter_count(from))
 				break;
-- 
2.39.0.rc1.256.g54fd8350bd-goog