Skip to contentSkip to conversation

mm: Check for NULL return values from allocating functions

PATCH In review Laura Abbott 2 participants 2 messages Oct 17, 2013 – Oct 18, 2013

Subsystems (2)
Memory management VFS
lore
Share & tools
lore

Laura Abbott

author
#0 lore raw

PATCH 1/1 mm: Check for NULL return values from allocating functions

A security audit revealed that several functions were not checking return value of allocation functions. These allocations may return NULL which may lead to NULL pointer dereferences and crashes or security concerns. Fix this by properly checking the return value and handling the error appropriately.

Signed-off-by Laura Abbott <[email protected]>
---
 fs/buffer.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)
fs/buffer.c +11 -6
index 4d74335..b53f863 100644
@@ -1561,6 +1561,9 @@ void create_empty_buffers(struct page *page,
 	struct buffer_head *bh, *head, *tail;
 
 	head = alloc_page_buffers(page, blocksize, 1);
+	if (head == NULL)
+		return;
+
 	bh = head;
 	do {
 		bh->b_state |= b_state;

@@ -3008,16 +3011,18 @@ int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
 	BUG_ON(buffer_unwritten(bh));
 
 	/*
-	 * Only clear out a write error when rewriting
-	 */
-	if (test_set_buffer_req(bh) && (rw & WRITE))
-		clear_buffer_write_io_error(bh);
-
-	/*
 	 * from here on down, it's all bio -- do the initial mapping,
 	 * submit_bio -> generic_make_request may further map this bio around
 	 */
 	bio = bio_alloc(GFP_NOIO, 1);
+	if (bio == NULL)
+		return -ENOMEM;
+
+	/*
+	 * Only clear out a write error when rewriting
+	 */
+	if (test_set_buffer_req(bh) && (rw & WRITE))
+		clear_buffer_write_io_error(bh);
 
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
Signature · 8 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
reviewer
#1 lore raw
Quoted 16 lines · Laura Abbott

A security audit revealed that several functions were not checking return value of allocation functions. These allocations may return NULL which may lead to NULL pointer dereferences and crashes or security concerns. Fix this by properly checking the return value and handling the error appropriately.

Signed-off-by Laura Abbott <[email protected]>
---
 fs/buffer.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)
fs/buffer.c +0 -0
index 4d74335..b53f863 100644

fs/buffer.c void create_empty_buffers(struct page *page, · line 1566

Quoted 6 lines
fs/buffer.c +3 -0
@@ -1561,6 +1561,9 @@ void create_empty_buffers(struct page *page,
 	struct buffer_head *bh, *head, *tail;
 
 	head = alloc_page_buffers(page, blocksize, 1);
+	if (head == NULL)
+		return;
+

This cannot happen. alloc_page_buffers() is called with retry == 1 and
thus it will loop until it gets the memory it wants.

Quoted 9 lines
fs/buffer.c +0 -2
 	bh = head;
 	do {
 		bh->b_state |= b_state;

@@ -3008,16 +3011,18 @@ int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
 	BUG_ON(buffer_unwritten(bh));
 
 	/*
-	 * Only clear out a write error when rewriting
-	 */

fs/buffer.c int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags) · line 3019

Quoted 10 lines
fs/buffer.c +2 -4
-	if (test_set_buffer_req(bh) && (rw & WRITE))
-		clear_buffer_write_io_error(bh);
-
-	/*
 	 * from here on down, it's all bio -- do the initial mapping,
 	 * submit_bio -> generic_make_request may further map this bio around
 	 */
 	bio = bio_alloc(GFP_NOIO, 1);
+	if (bio == NULL)
+		return -ENOMEM;

And the same is true here. If the gfp mask has __GFP_WAIT set (and
GFP_NOIO does have that), mempool_alloc() loops until it gets the memory. So I agree we might be missing some details in documentation but the code is correct.

Honza

Quoted 9 lines
fs/buffer.c +6 -0
+
+	/*
+	 * Only clear out a write error when rewriting
+	 */
+	if (test_set_buffer_req(bh) && (rw & WRITE))
+		clear_buffer_write_io_error(bh);
 
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
Signature · 8 lines
Jan Kara <[email protected]>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>