Re: [PATCH] fibmap: Reject negative addresses

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 3/18/19 11:10 AM, Carlos Maiolino wrote:
> FIBMAP receives an integer from userspace which is then implicitly
> converted into sector_t on ->bmap(). No check is made to ensure
> userspace didn't send a negative address, which can confuse the ->bmap
> interface, and return fuzzy addresses.

Nitpick on summary & comments,

I think this should be "negative logical block number?" because "address"
often means something completely different.  ;)

what is a "fuzzy address?"  What actually happens if a negative number
gets through?

> Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> ---

> I am not 100% sure if is there any reason for ->bmap interface to accept
> negative values, from the top of my mind, I really can't see a reason, if there
> is a reason why we don't check for negative addresses, I'd be happy to know,
> otherwise we should reject it.
> Cheers
>  fs/ioctl.c | 5 +++++
>  1 file changed, 5 insertions(+)
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index fef3a6bf7c78..e3a01c43adb4 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -64,6 +64,11 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
>  	res = get_user(block, p);
>  	if (res)
>  		return res;
> +
> +	/* No reason to query a negative block addr */

/* Invalid to query a negative block number */

Actually, the next 2 lines of code are not hard to understand, so probably
doesn't really even need a comment.

As for the change, it seems fine, I guess we don't document what the user
interface is supposed to be anywhere, sooo... go with the current signed int
implementation, and disallow anything that doesn't fit, makes sense to me.


> +	if (block < 0)
> +		return -EINVAL;
> +
>  	res = mapping->a_ops->bmap(mapping, block);
>  	return put_user(res, p);
>  }

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux