Re: [PATCH] ufs: use u64 for directory size in ufs_last_byte

[Jan Kara]

On Sat 18-07-26 19:03:07, YANXIN LI wrote:
> ufs1_read_inode() and ufs2_read_inode() copy the untrusted 64-bit
> on-disk size into inode->i_size.  ufs_last_byte() then truncates that
> value to an unsigned int before calculating the valid extent of a
> directory folio.
> 
> A directory size of exactly 4 GiB is truncated to zero.  In
> ufs_find_entry(), subtracting the requested record length from that zero
> forms an endpoint almost 4 GiB beyond the mapped folio.  A crafted UFS2
> image produces:
> 
>   BUG: KASAN: use-after-free in ufs_find_entry+0x583/0x6e0 [ufs]
>   Read of size 1
>   ...
>   ufs_find_entry
>   ufs_inode_by_name
>   ufs_lookup
>   __lookup_slow
>   path_lookupat
> 
> The KASAN classification reflects that the adjacent physical page was
> free; the source-level operation is an out-of-folio read.  A controlled
> UFS1 test with CONFIG_UFS_FS_WRITE enabled also made ufs_find_entry()
> return a directory entry from an adjacent anonymous page.  unlink() then
> passed that pointer to ufs_delete_entry(), which cleared the adjacent
> page's 3
> 2-bit d_ino.  This write result reproduced three out of three
> times.
> 
> UFS normally requires a privileged mount path.  A relevant boundary is
> a privileged automounter or image-processing service handling an
> attacker-supplied filesystem.  The write primitive additionally requires
> UFS1 to be mounted read-write with CONFIG_UFS_FS_WRITE enabled; the read
> is reachable with read-only UFS2.
> 
> Keep the intermediate size arithmetic at 64 bits so non-final pages are
> bounded at PAGE_SIZE.  With this change, the same controlled tests reach
> the real on-disk directory entry and produce no adjacent-page write or
> KASAN report.  A reproducer and full validation logs are available on
> request.
> 
> The vulnerable helper is present in v7.2-rc3, v7.1, v6.18.38, v6.12.95,
> v6.6.111, and upstream master at 1229e2e57a5c.  Runtime reproduction and
> fix validation were performed in an x86-64 QEMU guest running v7.2-rc3
> with generic KASAN.
> 
> Fixes: b71034e5e67d ("[PATCH] ufs: directory and page cache:
>  from blocks to pages")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: YANXIN LI <fadouse@xxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ufs/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c
> index e62fe5667..69e6d5535 100644
> --- a/fs/ufs/dir.c
> +++ b/fs/ufs/dir.c
> @@ -213,7 +213,7 @@ static void *ufs_get_folio(struct inode *dir, unsigned long n,
>  static unsigned
>  ufs_last_byte(struct inode *inode, unsigned long page_nr)
>  {
> -	unsigned last_byte = inode->i_size;
> +	u64 last_byte = inode->i_size;
>  
>  	last_byte -= page_nr << PAGE_SHIFT;
>  	if (last_byte > PAGE_SIZE)
> 
> base-commit: 1229e2e57a5c2980ccd457b9b53ea0eed5a22ab3
> -- 
> 2.54.0
> 




-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR