[PATCH v2] exfat: fix random stack corruption after get_block

[PATCH v2] exfat: fix random stack corruption after get_block

[Sungjong Seo]

When get_block is called with a buffer_head allocated on the stack, such
as do_mpage_readpage, stack corruption due to buffer_head UAF may occur in
the following race condition situation.

     <CPU 0>                      <CPU 1>
mpage_read_folio
  <<bh on stack>>
  do_mpage_readpage
    exfat_get_block
      bh_read
        __bh_read
	  get_bh(bh)
          submit_bh
          wait_on_buffer
                              ...
                              end_buffer_read_sync
                                __end_buffer_read_notouch
                                   unlock_buffer
          <<keep going>>
        ...
      ...
    ...
  ...
<<bh is not valid out of mpage_read_folio>>
   .
   .
another_function
  <<variable A on stack>>
                                   put_bh(bh)
                                     atomic_dec(bh->b_count)
  * stack corruption here *

This patch returns -EAGAIN if a folio does not have buffers when bh_read
needs to be called. By doing this, the caller can fallback to functions
like block_read_full_folio(), create a buffer_head in the folio, and then
call get_block again.

Let's do not call bh_read() with on-stack buffer_head.

Fixes: 11a347fb6cef ("exfat: change to get file size from DataLength")
Cc: stable@vger.kernel.org
Tested-by: Yeongjin Gil <youngjin.gil@samsung.com>
Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
---
v2:
 - clear_buffer_mapped if there is any errors.
 - remove unnecessary BUG_ON()
---
 fs/exfat/inode.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 96952d4acb50..f3fdba9f4d21 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -344,7 +344,8 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 			 * The block has been partially written,
 			 * zero the unwritten part and map the block.
 			 */
-			loff_t size, off, pos;
+			loff_t size, pos;
+			void *addr;
 
 			max_blocks = 1;
 
@@ -355,17 +356,41 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 			if (!bh_result->b_folio)
 				goto done;
 
+			/*
+			 * No buffer_head is allocated.
+			 * (1) bmap: It's enough to fill bh_result without I/O.
+			 * (2) read: The unwritten part should be filled with 0
+			 *           If a folio does not have any buffers,
+			 *           let's returns -EAGAIN to fallback to
+			 *           per-bh IO like block_read_full_folio().
+			 */
+			if (!folio_buffers(bh_result->b_folio)) {
+				err = -EAGAIN;
+				goto done;
+			}
+
 			pos = EXFAT_BLK_TO_B(iblock, sb);
 			size = ei->valid_size - pos;
-			off = pos & (PAGE_SIZE - 1);
+			addr = folio_address(bh_result->b_folio) +
+			       offset_in_folio(bh_result->b_folio, pos);
+
+			/* Check if bh->b_data points to proper addr in folio */
+			if (bh_result->b_data != addr) {
+				exfat_fs_error_ratelimit(sb,
+					"b_data(%p) != folio_addr(%p)",
+					bh_result->b_data, addr);
+				err = -EINVAL;
+				goto done;
+			}
 
-			folio_set_bh(bh_result, bh_result->b_folio, off);
+			/* Read a block */
 			err = bh_read(bh_result, 0);
 			if (err < 0)
-				goto unlock_ret;
+				goto done;
 
-			folio_zero_segment(bh_result->b_folio, off + size,
-					off + sb->s_blocksize);
+			/* Zero unwritten part of a block */
+			memset(bh_result->b_data + size, 0,
+			       bh_result->b_size - size);
 		} else {
 			/*
 			 * The range has not been written, clear the mapped flag
@@ -376,6 +401,8 @@ static int exfat_get_block(struct inode *inode, sector_t iblock,
 	}
 done:
 	bh_result->b_size = EXFAT_BLK_TO_B(max_blocks, sb);
+	if (err < 0)
+		clear_buffer_mapped(bh_result);
 unlock_ret:
 	mutex_unlock(&sbi->s_lock);
 	return err;
-- 
2.25.1

Re: [PATCH v2] exfat: fix random stack corruption after get_block

[Yuezhang.Mo]

Reviewed-by: Yuezhang Mo <Yuezhang.Mo@sony.com>

Re: [PATCH v2] exfat: fix random stack corruption after get_block

[Namjae Jeon]

On Fri, Mar 21, 2025 at 3:38 PM Sungjong Seo <sj1557.seo@samsung.com> wrote:
>
> When get_block is called with a buffer_head allocated on the stack, such
> as do_mpage_readpage, stack corruption due to buffer_head UAF may occur in
> the following race condition situation.
>
>      <CPU 0>                      <CPU 1>
> mpage_read_folio
>   <<bh on stack>>
>   do_mpage_readpage
>     exfat_get_block
>       bh_read
>         __bh_read
>           get_bh(bh)
>           submit_bh
>           wait_on_buffer
>                               ...
>                               end_buffer_read_sync
>                                 __end_buffer_read_notouch
>                                    unlock_buffer
>           <<keep going>>
>         ...
>       ...
>     ...
>   ...
> <<bh is not valid out of mpage_read_folio>>
>    .
>    .
> another_function
>   <<variable A on stack>>
>                                    put_bh(bh)
>                                      atomic_dec(bh->b_count)
>   * stack corruption here *
>
> This patch returns -EAGAIN if a folio does not have buffers when bh_read
> needs to be called. By doing this, the caller can fallback to functions
> like block_read_full_folio(), create a buffer_head in the folio, and then
> call get_block again.
>
> Let's do not call bh_read() with on-stack buffer_head.
>
> Fixes: 11a347fb6cef ("exfat: change to get file size from DataLength")
> Cc: stable@vger.kernel.org
> Tested-by: Yeongjin Gil <youngjin.gil@samsung.com>
> Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
Applied it to #dev.
Thanks!