linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: Check for NULL return values from allocating functions
@ 2013-10-17 14:49 Laura Abbott
  2013-10-18  8:39 ` Jan Kara
  0 siblings, 1 reply; 2+ messages in thread
From: Laura Abbott @ 2013-10-17 14:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-fsdevel, linux-kernel, 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 <lauraa@codeaurora.org>
---
 fs/buffer.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 4d74335..b53f863 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -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;
-- 
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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mm: Check for NULL return values from allocating functions
  2013-10-17 14:49 [PATCH] mm: Check for NULL return values from allocating functions Laura Abbott
@ 2013-10-18  8:39 ` Jan Kara
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2013-10-18  8:39 UTC (permalink / raw)
  To: Laura Abbott; +Cc: Andrew Morton, linux-mm, linux-fsdevel, linux-kernel

On Thu 17-10-13 07:49:34, Laura Abbott wrote:
> 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 <lauraa@codeaurora.org>
> ---
>  fs/buffer.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 4d74335..b53f863 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -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.

>  	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;
  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

> +
> +	/*
> +	 * 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;
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-10-18  8:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17 14:49 [PATCH] mm: Check for NULL return values from allocating functions Laura Abbott
2013-10-18  8:39 ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).