netdev - Re: [PATCH RESEND] ipv6: return with appropriate error code when sending RH0 using setsockopt()

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <48CF5C9D.5090200@cn.fujitsu.com>
Date:	Tue, 16 Sep 2008 15:13:33 +0800
From:	Shan Wei <shanwei@...fujitsu.com>
To:	David Miller <davem@...emloft.net>
CC:	brian.haley@...com, netdev@...r.kernel.org
Subject: Re: [PATCH RESEND] ipv6: return with appropriate error code when
 sending RH0 using setsockopt()

David Miller 写道:
> From: Shan Wei <shanwei@...fujitsu.com>
> Date: Thu, 04 Sep 2008 11:44:10 +0800
> 
>> Brian Haley 写道:
>>> Shan Wei wrote:
>>>> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
>>>> index 4e5eac3..7a58597 100644
>>>> --- a/net/ipv6/ipv6_sockglue.c
>>>> +++ b/net/ipv6/ipv6_sockglue.c
>>>> @@ -353,9 +353,10 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>>>>  			goto e_inval;
>>>>  
>>>>  		/* hop-by-hop / destination options are privileged option */
>>>> -		retv = -EPERM;
>>>> -		if (optname != IPV6_RTHDR && !capable(CAP_NET_RAW))
>>>> +		if (optname != IPV6_RTHDR && !capable(CAP_NET_RAW)) {
>>>> +			retv = -EPERM;
>>>>  			break;
>>>> +		}
>>> I'm not sure you need this since it doesn't actually change anything,
>>> setting the error before the check just seems like the style in this
>>> function.  It will get set to -EINVAL below...
>>>
>> For uncommon expection, if the check is true, we set the error. 
>> it will reduce the number of setting retv value.
> 
> Do you have some emotional attachment to this change? :-/
> 
> The compiler optimizer will make the transformation you claim is
> now possible, all by itself, if it is deemed more optimal.
> 
> Never make any claims about how many times something will or will not
> happen in the compiled output without consulting and providing
> examples of the resulting assembler.  Otherwise you are just guessing.
> 
> This is just a pointless change, and as Brian said it disagrees with
> the way this construct is made in the rest of this function.  So
> please take it out of your patch.
sorry for repling late.
maybe what i describe is not explicit.

in this function the style is like this:
case XX:
{
  retv = -EPROTO;
  if (condition1) break;
  ...
  retv = -EFAULT;
  if (condition2) break;
  ...
  retv = -EINVAL;
  if (condition3) break;
  ...
}
when the conditions are all false, the retv value is set three times.

but with the following style, the retv value is not set.
case XX:
{
  if (condition1){ retv = -EPROTO;break;}
  ...
  if (condition2){ retv = -EFAULT;break;}
  ...
  if (condition3){ retv = -EINVAL;break;}
  ...
}
Seting the retv value when expection is occurred almost don't improve
the performance, but it's a little better.

--
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