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@xxxxxxxxxx> 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@xxxxxxxxxx> > --- > 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
