[PATCH linux dev-4.7] drivers: fsi: Fix FSI core size checking user interfaces

[PATCH linux dev-4.7] drivers: fsi: Fix FSI core size checking user interfaces

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

Some potential for integer overflow and not checking signed offsets.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index d63a892..e13774f 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -90,10 +90,7 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
 int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
 		size_t size)
 {
-	if (addr > dev->size)
-		return -EINVAL;
-
-	if (addr + size > dev->size)
+	if (addr > dev->size || size > dev->size || addr > dev->size - size)
 		return -EINVAL;
 
 	return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
@@ -103,10 +100,7 @@ EXPORT_SYMBOL_GPL(fsi_device_read);
 int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
 		size_t size)
 {
-	if (addr > dev->size)
-		return -EINVAL;
-
-	if (addr + size > dev->size)
+	if (addr > dev->size || size > dev->size || addr > dev->size - size)
 		return -EINVAL;
 
 	return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
@@ -328,7 +322,7 @@ static ssize_t fsi_slave_sysfs_raw_read(struct file *file,
 	if (count != 4 || off & 0x3)
 		return -EINVAL;
 
-	if (off > 0xffffffff)
+	if (off > 0xfffffffc || off < 0)
 		return -EINVAL;
 
 	rc = fsi_slave_read(slave, off, buf, 4);
@@ -346,7 +340,7 @@ static ssize_t fsi_slave_sysfs_raw_write(struct file *file,
 	if (count != 4 || off & 0x3)
 		return -EINVAL;
 
-	if (off > 0xffffffff)
+	if (off > 0xfffffffc || off < 0)
 		return -EINVAL;
 
 	rc = fsi_slave_write(slave, off, buf, 4);
-- 
1.8.3.1

Re: [PATCH linux dev-4.7] drivers: fsi: Fix FSI core size checking user interfaces

[Jeremy Kerr]

Hi Eddie,

> Some potential for integer overflow and not checking signed offsets.

[...]

>  int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
>  		size_t size)
>  {
> -	if (addr > dev->size)
> -		return -EINVAL;
> -
> -	if (addr + size > dev->size)
> +	if (addr > dev->size || size > dev->size || addr > dev->size - size)
>  		return -EINVAL;

I liked the split conditional statements, but that's only personal
preference. Regardless, LGTM.

Acked-by: Jeremy Kerr <jk@ozlabs.org>

Cheers,


Jeremy

Re: [PATCH linux dev-4.7] drivers: fsi: Fix FSI core size checking user interfaces

[Joel Stanley]

On Wed, Feb 22, 2017 at 11:14 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Eddie,
>
>> Some potential for integer overflow and not checking signed offsets.
>
> [...]
>
>>  int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
>>               size_t size)
>>  {
>> -     if (addr > dev->size)
>> -             return -EINVAL;
>> -
>> -     if (addr + size > dev->size)
>> +     if (addr > dev->size || size > dev->size || addr > dev->size - size)
>>               return -EINVAL;
>
> I liked the split conditional statements, but that's only personal
> preference. Regardless, LGTM.

My personal preference is also to split them out. It makes them easier
to read, and there's less noise in the diff if the conditions need to
be updated.

I applied this as-is to dev-4.7 for now.

> Acked-by: Jeremy Kerr <jk@ozlabs.org>

Thanks for the review Jeremy.

Cheers,

Joel