diff mbox series

cifs: Fix use-after-free in rdata->read_into_pages()

Message ID 20230206011009.1126177-1-wangzhaolong1@huawei.com
State New
Headers show
Series cifs: Fix use-after-free in rdata->read_into_pages() | expand

Commit Message

Wang Zhaolong Feb. 6, 2023, 1:10 a.m. UTC
When the network status is unstable, use-after-free may occur when
read data from the server.

  BUG: KASAN: use-after-free in readpages_fill_pages+0x14c/0x7e0

  Call Trace:
   <TASK>
   dump_stack_lvl+0x38/0x4c
   print_report+0x16f/0x4a6
   kasan_report+0xb7/0x130
   readpages_fill_pages+0x14c/0x7e0
   cifs_readv_receive+0x46d/0xa40
   cifs_demultiplex_thread+0x121c/0x1490
   kthread+0x16b/0x1a0
   ret_from_fork+0x2c/0x50
   </TASK>

  Allocated by task 2535:
   kasan_save_stack+0x22/0x50
   kasan_set_track+0x25/0x30
   __kasan_kmalloc+0x82/0x90
   cifs_readdata_direct_alloc+0x2c/0x110
   cifs_readdata_alloc+0x2d/0x60
   cifs_readahead+0x393/0xfe0
   read_pages+0x12f/0x470
   page_cache_ra_unbounded+0x1b1/0x240
   filemap_get_pages+0x1c8/0x9a0
   filemap_read+0x1c0/0x540
   cifs_strict_readv+0x21b/0x240
   vfs_read+0x395/0x4b0
   ksys_read+0xb8/0x150
   do_syscall_64+0x3f/0x90
   entry_SYSCALL_64_after_hwframe+0x72/0xdc

  Freed by task 79:
   kasan_save_stack+0x22/0x50
   kasan_set_track+0x25/0x30
   kasan_save_free_info+0x2e/0x50
   __kasan_slab_free+0x10e/0x1a0
   __kmem_cache_free+0x7a/0x1a0
   cifs_readdata_release+0x49/0x60
   process_one_work+0x46c/0x760
   worker_thread+0x2a4/0x6f0
   kthread+0x16b/0x1a0
   ret_from_fork+0x2c/0x50

  Last potentially related work creation:
   kasan_save_stack+0x22/0x50
   __kasan_record_aux_stack+0x95/0xb0
   insert_work+0x2b/0x130
   __queue_work+0x1fe/0x660
   queue_work_on+0x4b/0x60
   smb2_readv_callback+0x396/0x800
   cifs_abort_connection+0x474/0x6a0
   cifs_reconnect+0x5cb/0xa50
   cifs_readv_from_socket.cold+0x22/0x6c
   cifs_read_page_from_socket+0xc1/0x100
   readpages_fill_pages.cold+0x2f/0x46
   cifs_readv_receive+0x46d/0xa40
   cifs_demultiplex_thread+0x121c/0x1490
   kthread+0x16b/0x1a0
   ret_from_fork+0x2c/0x50

The following function calls will cause UAF of the rdata pointer.

readpages_fill_pages
 cifs_read_page_from_socket
  cifs_readv_from_socket
   cifs_reconnect
    __cifs_reconnect
     cifs_abort_connection
      mid->callback() --> smb2_readv_callback
       queue_work(&rdata->work)  # if the worker completes first,
                                 # the rdata is freed
          cifs_readv_complete
            kref_put
              cifs_readdata_release
                kfree(rdata)
 return rdata->...               # UAF in readpages_fill_pages()

Similarly, this problem also occurs in the uncache_fill_pages().

Fix this by adjusts the order of condition judgment in the return
statement.

Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com>
---
 fs/cifs/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Steve French Feb. 7, 2023, 4:58 a.m. UTC | #1
added Cc:stable and acked-by Paulo

merged into cifs-2.6.git for-next pending testing

On Sun, Feb 5, 2023 at 7:11 PM ZhaoLong Wang <wangzhaolong1@huawei.com> wrote:
>
> When the network status is unstable, use-after-free may occur when
> read data from the server.
>
>   BUG: KASAN: use-after-free in readpages_fill_pages+0x14c/0x7e0
>
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x38/0x4c
>    print_report+0x16f/0x4a6
>    kasan_report+0xb7/0x130
>    readpages_fill_pages+0x14c/0x7e0
>    cifs_readv_receive+0x46d/0xa40
>    cifs_demultiplex_thread+0x121c/0x1490
>    kthread+0x16b/0x1a0
>    ret_from_fork+0x2c/0x50
>    </TASK>
>
>   Allocated by task 2535:
>    kasan_save_stack+0x22/0x50
>    kasan_set_track+0x25/0x30
>    __kasan_kmalloc+0x82/0x90
>    cifs_readdata_direct_alloc+0x2c/0x110
>    cifs_readdata_alloc+0x2d/0x60
>    cifs_readahead+0x393/0xfe0
>    read_pages+0x12f/0x470
>    page_cache_ra_unbounded+0x1b1/0x240
>    filemap_get_pages+0x1c8/0x9a0
>    filemap_read+0x1c0/0x540
>    cifs_strict_readv+0x21b/0x240
>    vfs_read+0x395/0x4b0
>    ksys_read+0xb8/0x150
>    do_syscall_64+0x3f/0x90
>    entry_SYSCALL_64_after_hwframe+0x72/0xdc
>
>   Freed by task 79:
>    kasan_save_stack+0x22/0x50
>    kasan_set_track+0x25/0x30
>    kasan_save_free_info+0x2e/0x50
>    __kasan_slab_free+0x10e/0x1a0
>    __kmem_cache_free+0x7a/0x1a0
>    cifs_readdata_release+0x49/0x60
>    process_one_work+0x46c/0x760
>    worker_thread+0x2a4/0x6f0
>    kthread+0x16b/0x1a0
>    ret_from_fork+0x2c/0x50
>
>   Last potentially related work creation:
>    kasan_save_stack+0x22/0x50
>    __kasan_record_aux_stack+0x95/0xb0
>    insert_work+0x2b/0x130
>    __queue_work+0x1fe/0x660
>    queue_work_on+0x4b/0x60
>    smb2_readv_callback+0x396/0x800
>    cifs_abort_connection+0x474/0x6a0
>    cifs_reconnect+0x5cb/0xa50
>    cifs_readv_from_socket.cold+0x22/0x6c
>    cifs_read_page_from_socket+0xc1/0x100
>    readpages_fill_pages.cold+0x2f/0x46
>    cifs_readv_receive+0x46d/0xa40
>    cifs_demultiplex_thread+0x121c/0x1490
>    kthread+0x16b/0x1a0
>    ret_from_fork+0x2c/0x50
>
> The following function calls will cause UAF of the rdata pointer.
>
> readpages_fill_pages
>  cifs_read_page_from_socket
>   cifs_readv_from_socket
>    cifs_reconnect
>     __cifs_reconnect
>      cifs_abort_connection
>       mid->callback() --> smb2_readv_callback
>        queue_work(&rdata->work)  # if the worker completes first,
>                                  # the rdata is freed
>           cifs_readv_complete
>             kref_put
>               cifs_readdata_release
>                 kfree(rdata)
>  return rdata->...               # UAF in readpages_fill_pages()
>
> Similarly, this problem also occurs in the uncache_fill_pages().
>
> Fix this by adjusts the order of condition judgment in the return
> statement.
>
> Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com>
> ---
>  fs/cifs/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 22dfc1f8b4f1..b8d1cbadb689 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -3889,7 +3889,7 @@ uncached_fill_pages(struct TCP_Server_Info *server,
>                 rdata->got_bytes += result;
>         }
>
> -       return rdata->got_bytes > 0 && result != -ECONNABORTED ?
> +       return result != -ECONNABORTED && rdata->got_bytes > 0 ?
>                                                 rdata->got_bytes : result;
>  }
>
> @@ -4665,7 +4665,7 @@ readpages_fill_pages(struct TCP_Server_Info *server,
>                 rdata->got_bytes += result;
>         }
>
> -       return rdata->got_bytes > 0 && result != -ECONNABORTED ?
> +       return result != -ECONNABORTED && rdata->got_bytes > 0 ?
>                                                 rdata->got_bytes : result;
>  }
>
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 22dfc1f8b4f1..b8d1cbadb689 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3889,7 +3889,7 @@  uncached_fill_pages(struct TCP_Server_Info *server,
 		rdata->got_bytes += result;
 	}
 
-	return rdata->got_bytes > 0 && result != -ECONNABORTED ?
+	return result != -ECONNABORTED && rdata->got_bytes > 0 ?
 						rdata->got_bytes : result;
 }
 
@@ -4665,7 +4665,7 @@  readpages_fill_pages(struct TCP_Server_Info *server,
 		rdata->got_bytes += result;
 	}
 
-	return rdata->got_bytes > 0 && result != -ECONNABORTED ?
+	return result != -ECONNABORTED && rdata->got_bytes > 0 ?
 						rdata->got_bytes : result;
 }