linux-kernel - [PATCH] fs: avoid NULL pointer dereference for nobh on write error

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

Message-Id: <20171109214913.7814-1-jglisse@redhat.com>
Date:   Thu,  9 Nov 2017 16:49:13 -0500
From:   jglisse@...hat.com
To:     linux-kernel@...r.kernel.org
Cc:     Jérôme Glisse <jglisse@...hat.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org, stable@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH] fs: avoid NULL pointer dereference for nobh on write error

From: Jérôme Glisse <jglisse@...hat.com>

(Result of code inspection only, i do not have a bug, nor know a bug
that would be explain by this issue. Is there a kernel trace database
one can query for that ?)

When fs is mounted with nobh temporary buffer_head are created during
write and they are only associated with the page when a filesystem
error happen. When this happen nobh_write_begin() or nobh_write_end()
call attach_nobh_buffers() which expect that provided buffer_head
list is circular (ie tail entry point to head entry). If it is not
the case it will dereference the last pointer in the list which is
NULL (last item in the list point to NULL see alloc_page_buffers())
and thus SEGFAULT.

Hence nobh_write_begin() must make the buffer_head list circular as
alloc_page_buffers() is not responsible for doing that.

(This might worth including in 4.14 as i don't think  it can regress
anything but i am not a filesystem expert).

Note i did not make the list circular inside attach_nobh_buffers()
as some patchset i am working on also expect the list to be circular
no matter what. But if people are more confortable with me doing
that in my patchset the fix can be move to attach_nobh_buffers().

Signed-off-by: Jérôme Glisse <jglisse@...hat.com>
Cc: Al Viro <viro@...iv.linux.org.uk>
Cc: linux-fsdevel@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Cc: stable@...r.kernel.org
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
---
 fs/buffer.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 170df856bdb9..6bc47c11d6ac 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2598,7 +2598,7 @@ int nobh_write_begin(struct address_space *mapping,
 	struct inode *inode = mapping->host;
 	const unsigned blkbits = inode->i_blkbits;
 	const unsigned blocksize = 1 << blkbits;
-	struct buffer_head *head, *bh;
+	struct buffer_head *head, *bh, *tail;
 	struct page *page;
 	pgoff_t index;
 	unsigned from, to;
@@ -2643,6 +2643,13 @@ int nobh_write_begin(struct address_space *mapping,
 		ret = -ENOMEM;
 		goto out_release;
 	}
+	/* We need to make buffer_head list circular to avoid NULL SEGFAULT */
+	bh = head;
+	do {
+		tail = bh;
+		bh = bh->b_this_page;
+	} while (bh);
+	tail->b_this_page = head;

 	block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits);

-- 
2.13.6