diff mbox series

Fixed null-ptr-deref Read in drop_buffers

Message ID 20241108023717.8613-1-sarvesh20123@gmail.com (mailing list archive)
State New
Headers show
Series Fixed null-ptr-deref Read in drop_buffers | expand

Commit Message

Saru2003 Nov. 8, 2024, 2:37 a.m. UTC
Signed-off-by: Saru2003 <sarvesh20123@gmail.com>
---
 fs/buffer.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Al Viro Nov. 8, 2024, 3:46 a.m. UTC | #1
On Fri, Nov 08, 2024 at 08:07:17AM +0530, Saru2003 wrote:
> Signed-off-by: Saru2003 <sarvesh20123@gmail.com>
> ---
>  fs/buffer.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 1fc9a50def0b..e32420d8b9e3 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2888,14 +2888,23 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
>  	struct buffer_head *head = folio_buffers(folio);
>  	struct buffer_head *bh;
>  
> +	if (!head) {
> +		goto failed;
> +	}

Which caller can hit that?

>  	bh = head;
>  	do {
> +		if (!bh)
> +			goto failed;

Huh?  How the hell can _that_ happen?  "Somehow got called for a folio
without associated buffer_heads" is one thing, but this would require
an obvious memory corruption; a NULL forward pointer in the middle of
a cyclic list.

>  		if (buffer_busy(bh))
>  			goto failed;
>  		bh = bh->b_this_page;
>  	} while (bh != head);
>  
>  	do {
> +		if (!bh)
> +			goto failed;

NAK.  In the best case you are papering over unrelated memory corruption.

Please, explain what's going on.  Random checks for pointers being NULL
are not magical unicorn piss to be sprinkled all over the place...

And "fixed null-pointer-deref" does *not* work as an explanation - not
with otherwise empty commit message.
diff mbox series

Patch

diff --git a/fs/buffer.c b/fs/buffer.c
index 1fc9a50def0b..e32420d8b9e3 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2888,14 +2888,23 @@  drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
 	struct buffer_head *head = folio_buffers(folio);
 	struct buffer_head *bh;
 
+	if (!head) {
+		goto failed;
+	}
+
 	bh = head;
 	do {
+		if (!bh)
+			goto failed;
 		if (buffer_busy(bh))
 			goto failed;
 		bh = bh->b_this_page;
 	} while (bh != head);
 
 	do {
+		if (!bh)
+			goto failed;
+
 		struct buffer_head *next = bh->b_this_page;
 
 		if (bh->b_assoc_map)