[bug report] btrfs: new ioctl TREE_SEARCH_V2

[bug report] btrfs: new ioctl TREE_SEARCH_V2

[Dan Carpenter]

Hello Gerhard Heift,

The patch cc68a8a5a433: "btrfs: new ioctl TREE_SEARCH_V2" from Jan
30, 2014 (linux-next), leads to the following Smatch static checker
warning:

	fs/btrfs/ioctl.c:1787 btrfs_ioctl_tree_search_v2()
	warn: not copying enough bytes for '&uarg->buf_size' (8 vs 4 bytes)

fs/btrfs/ioctl.c
    1760 static noinline int btrfs_ioctl_tree_search_v2(struct inode *inode,
    1761                                                void __user *argp)
    1762 {
    1763         struct btrfs_ioctl_search_args_v2 __user *uarg = argp;
    1764         struct btrfs_ioctl_search_args_v2 args;
    1765         int ret;
    1766         size_t buf_size;
    1767         const size_t buf_limit = SZ_16M;
    1768 
    1769         if (!capable(CAP_SYS_ADMIN))
    1770                 return -EPERM;
    1771 
    1772         /* copy search header and buffer size */
    1773         if (copy_from_user(&args, uarg, sizeof(args)))
    1774                 return -EFAULT;
    1775 
    1776         buf_size = args.buf_size;
    1777 
    1778         /* limit result size to 16MB */
    1779         if (buf_size > buf_limit)
    1780                 buf_size = buf_limit;
    1781 
    1782         ret = search_ioctl(inode, &args.key, &buf_size,
    1783                            (char __user *)(&uarg->buf[0]));
    1784         if (ret == 0 && copy_to_user(&uarg->key, &args.key, sizeof(args.key)))
    1785                 ret = -EFAULT;
    1786         else if (ret == -EOVERFLOW &&
--> 1787                 copy_to_user(&uarg->buf_size, &buf_size, sizeof(buf_size)))

uarg->buf_size is a u64 but we are copying sizeof(unsigned long) bytes
so on 32 bit systems that's not enough.  It probably works fine on
little endian 32 bit systems, but on big endian 32 bit systems it won't.

    1788                 ret = -EFAULT;
    1789 
    1790         return ret;
    1791 }

regards,
dan carpenter

Re: [bug report] btrfs: new ioctl TREE_SEARCH_V2

[Filipe Manana]

On Fri, Oct 13, 2023 at 8:52 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hello Gerhard Heift,
>
> The patch cc68a8a5a433: "btrfs: new ioctl TREE_SEARCH_V2" from Jan
> 30, 2014 (linux-next), leads to the following Smatch static checker
> warning:
>
>         fs/btrfs/ioctl.c:1787 btrfs_ioctl_tree_search_v2()
>         warn: not copying enough bytes for '&uarg->buf_size' (8 vs 4 bytes)
>
> fs/btrfs/ioctl.c
>     1760 static noinline int btrfs_ioctl_tree_search_v2(struct inode *inode,
>     1761                                                void __user *argp)
>     1762 {
>     1763         struct btrfs_ioctl_search_args_v2 __user *uarg = argp;
>     1764         struct btrfs_ioctl_search_args_v2 args;
>     1765         int ret;
>     1766         size_t buf_size;
>     1767         const size_t buf_limit = SZ_16M;
>     1768
>     1769         if (!capable(CAP_SYS_ADMIN))
>     1770                 return -EPERM;
>     1771
>     1772         /* copy search header and buffer size */
>     1773         if (copy_from_user(&args, uarg, sizeof(args)))
>     1774                 return -EFAULT;
>     1775
>     1776         buf_size = args.buf_size;
>     1777
>     1778         /* limit result size to 16MB */
>     1779         if (buf_size > buf_limit)
>     1780                 buf_size = buf_limit;
>     1781
>     1782         ret = search_ioctl(inode, &args.key, &buf_size,
>     1783                            (char __user *)(&uarg->buf[0]));
>     1784         if (ret == 0 && copy_to_user(&uarg->key, &args.key, sizeof(args.key)))
>     1785                 ret = -EFAULT;
>     1786         else if (ret == -EOVERFLOW &&
> --> 1787                 copy_to_user(&uarg->buf_size, &buf_size, sizeof(buf_size)))
>
> uarg->buf_size is a u64 but we are copying sizeof(unsigned long) bytes
> so on 32 bit systems that's not enough.  It probably works fine on
> little endian 32 bit systems, but on big endian 32 bit systems it won't.

Thanks for the report. I've just sent a fix for this:

https://lore.kernel.org/linux-btrfs/44cfbc3f3ee2465d776ce6926c6f1cece2511325.1697187887.git.fdmanana@suse.com/

>
>     1788                 ret = -EFAULT;
>     1789
>     1790         return ret;
>     1791 }
>
> regards,
> dan carpenter