Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Yicong Yang' <yangyicong@hisilicon.com>,
        "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
CC:     "akinobu.mita@gmail.com" <akinobu.mita@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linuxarm@huawei.com" <linuxarm@huawei.com>,
        "prime.zeng@huawei.com" <prime.zeng@huawei.com>
Subject: RE: [PATCH v2] libfs: fix error cast of negative value in
 simple_attr_write()
Thread-Topic: [PATCH v2] libfs: fix error cast of negative value in
 simple_attr_write()
Thread-Index: AQHWuaN2HYrg6O40uUKH2XiBcGhEHanF7BWQ
Date:   Fri, 13 Nov 2020 11:27:41 +0000
Message-ID: <accad76be7dd4b20a3206bbb4ee86688@AcuMS.aculab.com>
References: <1605261369-551-1-git-send-email-yangyicong@hisilicon.com>
In-Reply-To: <1605261369-551-1-git-send-email-yangyicong@hisilicon.com>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Yicong Yang
> Sent: 13 November 2020 09:56
> The attr->set() receive a value of u64, but simple_strtoll() is used
> for doing the conversion. It will lead to the error cast if user inputs
> a negative value.
> 
> Use kstrtoull() instead of simple_strtoll() to convert a string got
> from the user to an unsigned value. The former will return '-EINVAL' if
> it gets a negetive value, but the latter can't handle the situation
> correctly.
> 
> Fixes: f7b88631a897 ("fs/libfs.c: fix simple_attr_write() on 32bit machines")
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> Change since v1:
> - address the compile warning for non-64 bit platform
> 
>  fs/libfs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index fc34361..3a0d99c 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -977,7 +977,9 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
>  		goto out;
> 
>  	attr->set_buf[size] = '\0';
> -	val = simple_strtoll(attr->set_buf, NULL, 0);
> +	ret = kstrtoull(attr->set_buf, 0, (unsigned long long *)&val);

That cast is horrid.
Casting 'pointer to integer' types is just asking for trouble.
You either need to change the type of 'val' or use an
intermediary variable of the correct type.

	David

> +	if (ret)
> +		goto out;
>  	ret = attr->set(attr->data, val);
>  	if (ret == 0)
>  		ret = len; /* on success, claim we got the whole input */
> --
> 2.8.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)