netdev - Re: [PATCH][CAN]: Fix copy_from_user() results interpretation.

Open Source and information security mailing list archives

Message-ID: <4812E73A.6060104@hartkopp.net>
Date:	Sat, 26 Apr 2008 10:26:34 +0200
From:	Oliver Hartkopp <oliver@...tkopp.net>
To:	Sam Ravnborg <sam@...nborg.org>, David Miller <davem@...emloft.net>
CC:	Wolfgang Grandegger <wg@...ndegger.com>,
	"Thuermann, Urs, Dr. \(K-EFFI/I\)" <urs.thuermann@...kswagen.de>,
	Linux Netdev List <netdev@...r.kernel.org>,
	socketcan-core@...ts.berlios.de,
	Oliver Hartkopp <oliver.hartkopp@...kswagen.de>,
	Pavel Emelyanov <xemul@...nvz.org>
Subject: Re: [PATCH][CAN]: Fix copy_from_user() results interpretation.

Sam Ravnborg wrote:
> On Sat, Apr 26, 2008 at 08:19:31AM +0200, Wolfgang Grandegger wrote:
>   
>> What about removing the assignment "err =" in that case.
>>     
> Preferred.
>
> See sample patch below (made on top of -linus
> so it does likely not apply but made it only to
> see the difference anyway).
>   

I can definitely follow Daves remarks regarding the ability to review 
local bugfixes. In this case two commits should be used.

Btw. the cleanup an removal of the err variable usage in the pointed 
cases is a good suggestion. So even when the two latest changes in Sam's 
patch are not that obvious to be able to be reviewed locally i would 
like to ack this patch.

It should apply fine on Linus' tree and the current net-2.6.

Thanks to all of you & have a nice weekend.

Oliver

Acked-by: Oliver Hartkopp <oliver@...tkopp.net>

> diffstat:
>  net/can/raw.c          |   21 ++++++++++-----------
>  2 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 201cbfc..69877b8 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -435,15 +435,13 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>  			if (!filter)
>  				return -ENOMEM;
>  
> -			err = copy_from_user(filter, optval, optlen);
> -			if (err) {
> +			if (copy_from_user(filter, optval, optlen)) {
>  				kfree(filter);
> -				return err;
> +				return -EFAULT;
>  			}
>  		} else if (count == 1) {
> -			err = copy_from_user(&sfilter, optval, optlen);
> -			if (err)
> -				return err;
> +			if (copy_from_user(&sfilter, optval, optlen))
> +				return -EFAULT;
>  		}
>  
>  		lock_sock(sk);
> @@ -493,9 +491,8 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>  		if (optlen != sizeof(err_mask))
>  			return -EINVAL;
>  
> -		err = copy_from_user(&err_mask, optval, optlen);
> -		if (err)
> -			return err;
> +		if (copy_from_user(&err_mask, optval, optlen))
> +			return -EFAULT;
>  
>  		err_mask &= CAN_ERR_MASK;
>  
> @@ -531,7 +528,8 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>  		if (optlen != sizeof(ro->loopback))
>  			return -EINVAL;
>  
> -		err = copy_from_user(&ro->loopback, optval, optlen);
> +		if (copy_from_user(&ro->loopback, optval, optlen))
> +			return -EFAULT;
>  
>  		break;
>  
> @@ -539,7 +537,8 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>  		if (optlen != sizeof(ro->recv_own_msgs))
>  			return -EINVAL;
>  
> -		err = copy_from_user(&ro->recv_own_msgs, optval, optlen);
> +		if (copy_from_user(&ro->recv_own_msgs, optval, optlen))
> +			return -EFAULT;
>  
>  		break;
>  
> _______________________________________________
> Socketcan-core mailing list
> Socketcan-core@...ts.berlios.de
> https://lists.berlios.de/mailman/listinfo/socketcan-core
>   

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives