Re: [PATCH v2] cachefiles: Fix KASAN slab-out-of-bounds in cachefiles_set_volume_xattr

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2022-04-05 at 10:28 -0400, Dave Wysochanski wrote:
> Use the actual length of volume coherency data when setting the
> xattr to avoid the following KASAN report.
> 
>  BUG: KASAN: slab-out-of-bounds in cachefiles_set_volume_xattr+0xa0/0x350 [cachefiles]
>  Write of size 4 at addr ffff888101e02af4 by task kworker/6:0/1347
> 
>  CPU: 6 PID: 1347 Comm: kworker/6:0 Kdump: loaded Not tainted 5.18.0-rc1-nfs-fscache-netfs+ #13
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-4.fc34 04/01/2014
>  Workqueue: events fscache_create_volume_work [fscache]
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x45/0x5a
>   print_report.cold+0x5e/0x5db
>   ? __lock_text_start+0x8/0x8
>   ? cachefiles_set_volume_xattr+0xa0/0x350 [cachefiles]
>   kasan_report+0xab/0x120
>   ? cachefiles_set_volume_xattr+0xa0/0x350 [cachefiles]
>   kasan_check_range+0xf5/0x1d0
>   memcpy+0x39/0x60
>   cachefiles_set_volume_xattr+0xa0/0x350 [cachefiles]
>   cachefiles_acquire_volume+0x2be/0x500 [cachefiles]
>   ? __cachefiles_free_volume+0x90/0x90 [cachefiles]
>   fscache_create_volume_work+0x68/0x160 [fscache]
>   process_one_work+0x3b7/0x6a0
>   worker_thread+0x2c4/0x650
>   ? process_one_work+0x6a0/0x6a0
>   kthread+0x16c/0x1a0
>   ? kthread_complete_and_exit+0x20/0x20
>   ret_from_fork+0x22/0x30
>   </TASK>
> 
>  Allocated by task 1347:
>   kasan_save_stack+0x1e/0x40
>   __kasan_kmalloc+0x81/0xa0
>   cachefiles_set_volume_xattr+0x76/0x350 [cachefiles]
>   cachefiles_acquire_volume+0x2be/0x500 [cachefiles]
>   fscache_create_volume_work+0x68/0x160 [fscache]
>   process_one_work+0x3b7/0x6a0
>   worker_thread+0x2c4/0x650
>   kthread+0x16c/0x1a0
>   ret_from_fork+0x22/0x30
> 
>  The buggy address belongs to the object at ffff888101e02af0
>  which belongs to the cache kmalloc-8 of size 8
>  The buggy address is located 4 bytes inside of
>  8-byte region [ffff888101e02af0, ffff888101e02af8)
> 
>  The buggy address belongs to the physical page:
>  page:00000000a2292d70 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x101e02
>  flags: 0x17ffffc0000200(slab|node=0|zone=2|lastcpupid=0x1fffff)
>  raw: 0017ffffc0000200 0000000000000000 dead000000000001 ffff888100042280
>  raw: 0000000000000000 0000000080660066 00000001ffffffff 0000000000000000
>  page dumped because: kasan: bad access detected
> 
>  Memory state around the buggy address:
>  ffff888101e02980: fc 00 fc fc fc fc 00 fc fc fc fc 00 fc fc fc fc
>  ffff888101e02a00: 00 fc fc fc fc 00 fc fc fc fc 00 fc fc fc fc 00
>  >ffff888101e02a80: fc fc fc fc 00 fc fc fc fc 00 fc fc fc fc 04 fc
>                                                             ^
>  ffff888101e02b00: fc fc fc 00 fc fc fc fc 00 fc fc fc fc 00 fc fc
>  ffff888101e02b80: fc fc 00 fc fc fc fc 00 fc fc fc fc 00 fc fc fc
>  ==================================================================
> 
> Fixes: 413a4a6b0b55 "cachefiles: Fix volume coherency attribute"
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  fs/cachefiles/xattr.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c
> index 35465109d9c4..f13b642c1d14 100644
> --- a/fs/cachefiles/xattr.c
> +++ b/fs/cachefiles/xattr.c
> @@ -198,8 +198,7 @@ bool cachefiles_set_volume_xattr(struct cachefiles_volume *volume)
>  
>  	_enter("%x,#%d", volume->vcookie->debug_id, len);
>  
> -	len += sizeof(*buf);
> -	buf = kmalloc(len, GFP_KERNEL);
> +	buf = kmalloc(sizeof(*buf) + len, GFP_KERNEL);
>  	if (!buf)
>  		return false;
>  	buf->reserved = cpu_to_be32(0);

I hit this bug earlier today too.

Reviewed-and-Tested-by: Jeff Layton <jlayton@kernel.org>