Skip to contentSkip to conversation

Btrfs: fix null pointer dereference on compressed write path error

PATCH Superseded [email protected] 3 participants 4 messages Oct 12, 2018 – Oct 16, 2018 Btrfs file system

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 Btrfs: fix null pointer dereference on compressed write path error

From: Filipe Manana <[email protected]>

At inode.c:compress_file_range(), under the "free_pages_out" label, we can end up dereferencing the "pages" pointer when it has a NULL value. This case happens when "start" has a value of 0 and we fail to allocate memory for the "pages" pointer. When that happens we jump to the "cont" label and then enter the "if (start == 0)" branch where we immediately call the cow_file_range_inline() function. If that function returns an error (like -ENOMEM for example) we jump to the "free_pages_out" label and then access "pages[i]" leading to a NULL pointer dereference, since "nr_pages" has a value greater than zero at that point.

Fix this by setting "nr_pages" to 0 when we fail to allocate memory for the "pages" pointer.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201119

Fixes 771ed689d2cd ("Btrfs: Optimize compressed writeback and reads")
Signed-off-by Filipe Manana <[email protected]>
---
 fs/btrfs/inode.c | 1 +
 1 file changed, 1 insertion(+)
fs/btrfs/inode.c +1 -0
index 66c6c4103d2f..d6b61b1facdd 100644
@@ -503,6 +503,7 @@ static noinline void compress_file_range(struct inode *inode,
 		pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
 		if (!pages) {
 			/* just bail out to the uncompressed code */
+			nr_pages = 0;
 			goto cont;
 		}
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH v2 1/1 Btrfs: fix null pointer dereference on compressed write path error

From: Filipe Manana <[email protected]>

At inode.c:compress_file_range(), under the "free_pages_out" label, we can end up dereferencing the "pages" pointer when it has a NULL value. This case happens when "start" has a value of 0 and we fail to allocate memory for the "pages" pointer. When that happens we jump to the "cont" label and then enter the "if (start == 0)" branch where we immediately call the cow_file_range_inline() function. If that function returns 0 (success creating an inline extent) or an error (like -ENOMEM for example) we jump to the "free_pages_out" label and then access "pages[i]" leading to a NULL pointer dereference, since "nr_pages" has a value greater than zero at that point.

Fix this by setting "nr_pages" to 0 when we fail to allocate memory for the "pages" pointer.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201119

Fixes 771ed689d2cd ("Btrfs: Optimize compressed writeback and reads")
Signed-off-by Filipe Manana <[email protected]>

---

V2: Updated changelog.

 fs/btrfs/inode.c | 1 +
 1 file changed, 1 insertion(+)
fs/btrfs/inode.c +1 -0
index 66c6c4103d2f..d6b61b1facdd 100644
@@ -503,6 +503,7 @@ static noinline void compress_file_range(struct inode *inode,
 		pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
 		if (!pages) {
 			/* just bail out to the uncompressed code */
+			nr_pages = 0;
 			goto cont;
 		}
Signature · 1 line
2.11.0
Reviewed-by on v2 1/1
#2 lore raw
Quoted 14 lines · On Fri, Oct 12, 2018 at 4:38 PM <[email protected]> wrote:

From: Filipe Manana <[email protected]>

At inode.c:compress_file_range(), under the "free_pages_out" label, we can end up dereferencing the "pages" pointer when it has a NULL value. This case happens when "start" has a value of 0 and we fail to allocate memory for the "pages" pointer. When that happens we jump to the "cont" label and then enter the "if (start == 0)" branch where we immediately call the cow_file_range_inline() function. If that function returns 0 (success creating an inline extent) or an error (like -ENOMEM for example) we jump to the "free_pages_out" label and then access "pages[i]" leading to a NULL pointer dereference, since "nr_pages" has a value greater than zero at that point.

Quoted 3 lines

Fix this by setting "nr_pages" to 0 when we fail to allocate memory for the "pages" pointer.

Looks good.

Reviewed-by Liu Bo <[email protected]>

thanks,
liubo

Quoted 25 lines

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201119

Fixes 771ed689d2cd ("Btrfs: Optimize compressed writeback and reads")
Signed-off-by Filipe Manana <[email protected]>

---

V2: Updated changelog.

 fs/btrfs/inode.c | 1 +
 1 file changed, 1 insertion(+)
fs/btrfs/inode.c +1 -1
index 66c6c4103d2f..d6b61b1facdd 100644
@@ -503,6 +503,7 @@ static noinline void compress_file_range(struct inode *inode,
                pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
                if (!pages) {
                        /* just bail out to the uncompressed code */
+                       nr_pages = 0;
                        goto cont;
                }
 
--

2.11.0

#3 lore raw
In reply to [email protected]
Quoted 13 lines · On Sat, Oct 13, 2018 at 12:37:25AM +0100, [email protected] wrote:

From: Filipe Manana <[email protected]>

At inode.c:compress_file_range(), under the "free_pages_out" label, we can end up dereferencing the "pages" pointer when it has a NULL value. This case happens when "start" has a value of 0 and we fail to allocate memory for the "pages" pointer. When that happens we jump to the "cont" label and then enter the "if (start == 0)" branch where we immediately call the cow_file_range_inline() function. If that function returns 0 (success creating an inline extent) or an error (like -ENOMEM for example) we jump to the "free_pages_out" label and then access "pages[i]" leading to a NULL pointer dereference, since "nr_pages" has a value greater than zero at that point.

Quoted 6 lines

Fix this by setting "nr_pages" to 0 when we fail to allocate memory for the "pages" pointer.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201119

Fixes 771ed689d2cd ("Btrfs: Optimize compressed writeback and reads")
Signed-off-by Filipe Manana <[email protected]>

Added to misc-next, thanks.