Re: [PATCH] add missing checks of __copy_to_user return value in i2o_config.c

[Christoph Hellwig <hch@infradead.org>]

On Tue, Oct 05, 2004 at 11:43:54PM +0200, Jesper Juhl wrote:
> 
> This patch fixes up the following :
> 
>   CC      drivers/message/i2o/i2o_config.o
> include/asm/uaccess.h: In function `i2o_cfg_getiops':
> drivers/message/i2o/i2o_config.c:190: warning: ignoring return value of `__copy_to_user', declared with attribute warn_unused_result
> include/asm/uaccess.h: In function `i2o_cfg_swul':
> drivers/message/i2o/i2o_config.c:477: warning: ignoring return value of `__copy_to_user', declared with attribute warn_unused_result
> 
> Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
> 
> diff -up linux-2.6.9-rc3-bk5-orig/drivers/message/i2o/i2o_config.c linux-2.6.9-rc3-bk5/drivers/message/i2o/i2o_config.c
> --- linux-2.6.9-rc3-bk5-orig/drivers/message/i2o/i2o_config.c	2004-09-30 05:05:40.000000000 +0200
> +++ linux-2.6.9-rc3-bk5/drivers/message/i2o/i2o_config.c	2004-10-05 23:32:43.000000000 +0200
> @@ -187,7 +187,8 @@ static int i2o_cfg_getiops(unsigned long
>  	list_for_each_entry(c, &i2o_controllers, list)
>  	    tmp[c->unit] = 1;
>  
> -	__copy_to_user(user_iop_table, tmp, MAX_I2O_CONTROLLERS);
> +	if (__copy_to_user(user_iop_table, tmp, MAX_I2O_CONTROLLERS))
> +		return -EFAULT;

should be copy_to_user (with return value checked) and the
access_ok above should be removed.

>  	return 0;
>  };
> @@ -474,7 +475,9 @@ static int i2o_cfg_swul(unsigned long ar
>  		return status;
>  	}
>  
> -	__copy_to_user(kxfer.buf, buffer.virt, fragsize);
> +	if (__copy_to_user(kxfer.buf, buffer.virt, fragsize))
> +		return -EFAULT;
> +
>  	i2o_dma_free(&c->pdev->dev, &buffer);

you're adding a leak here,and again please use copy_to_user and remove
the access_ok abov