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 <[email protected]>
---
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
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 <[email protected]> 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 <[email protected]>
> ---
> 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
>
--
Thanks,
Steve