| 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
| ||
|
Date: Fri, 15 May 2015 11:27:25 -0400
From: Tom Herbert <tom@...bertland.com>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: Eric Dumazet <eric.dumazet@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: fix two sparse errors
On Fri, May 15, 2015 at 10:35 AM, Sabrina Dubroca <sd@...asysnail.net> wrote:
> Hi Eric,
>
> 2015-05-15, 05:48:07 -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@...gle.com>
>>
>> First one in __skb_checksum_validate_complete() fixes the following
>> (and other callers)
>>
>> make C=2 CF=-D__CHECK_ENDIAN__ net/ipv4/tcp_ipv4.o
>> CHECK net/ipv4/tcp_ipv4.c
>> include/linux/skbuff.h:3052:24: warning: incorrect type in return expression (different base types)
>> include/linux/skbuff.h:3052:24: expected restricted __sum16
>> include/linux/skbuff.h:3052:24: got int
>>
>> Second is fixing gso_make_checksum() :
>>
>> CHECK net/ipv4/gre_offload.c
>> include/linux/skbuff.h:3360:14: warning: incorrect type in assignment (different base types)
>> include/linux/skbuff.h:3360:14: expected unsigned short [unsigned] [usertype] csum
>> include/linux/skbuff.h:3360:14: got restricted __sum16
>> include/linux/skbuff.h:3365:16: warning: incorrect type in return expression (different base types)
>> include/linux/skbuff.h:3365:16: expected restricted __sum16
>> include/linux/skbuff.h:3365:16: got unsigned short [unsigned] [usertype] csum
>>
>>
>> Fixes: 5a21232983aa7 ("net: Support for csum_bad in skbuff")
>> Fixes: 7e2b10c1e52ca ("net: Support for multiple checksums with gso")
>> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>> CC: Tom Herbert <tom@...bertland.com>
>> ---
>
> That's very similar to what I submitted in February:
> https://patchwork.ozlabs.org/patch/437332/
>
> I have a patch to make __skb_checksum_validate_complete return a bool,
> since that's all the callers care about (it also needs modifications
> at some call sites, not included here).
>
> After that, maybe we should also reverse the logic so that "validate"
> functions return true when the csum is valid.
>
>
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index f83aa6568cbf..23dc1ccb28f0 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3034,24 +3034,23 @@ static inline void skb_checksum_complete_unset(struct sk_buff *skb)
> /* Validate (init) checksum based on checksum complete.
> *
> * Return values:
> - * 0: checksum is validated or try to in skb_checksum_complete. In the latter
> - * case the ip_summed will not be CHECKSUM_UNNECESSARY and the pseudo
> - * checksum is stored in skb->csum for use in __skb_checksum_complete
> - * non-zero: value of invalid checksum
> - *
> + * false: checksum is validated or try to in skb_checksum_complete. In the
> + * latter case the ip_summed will not be CHECKSUM_UNNECESSARY and the
> + * pseudo checksum is stored in skb->csum for use in __skb_checksum_complete
> + * true: invalid checksum
> */
> -static inline __sum16 __skb_checksum_validate_complete(struct sk_buff *skb,
> - bool complete,
> - __wsum psum)
> +static inline bool __skb_checksum_validate_complete(struct sk_buff *skb,
> + bool complete,
> + __wsum psum)
> {
> if (skb->ip_summed == CHECKSUM_COMPLETE) {
> if (!csum_fold(csum_add(psum, skb->csum))) {
> skb->csum_valid = 1;
> - return 0;
> + return false;
> }
> } else if (skb->csum_bad) {
> /* ip_summed == CHECKSUM_NONE in this case */
> - return 1;
> + return true;
> }
>
> skb->csum = psum;
> @@ -3061,10 +3060,10 @@ static inline __sum16 __skb_checksum_validate_complete(struct sk_buff *skb,
>
> csum = __skb_checksum_complete(skb);
> skb->csum_valid = !csum;
> - return csum;
> + return !!csum;
> }
>
> - return 0;
> + return false;
Hi Sabrina,
We returned the checksum value since that may be of interest to the
caller at some point.
> }
>
> static inline __wsum null_compute_pseudo(struct sk_buff *skb, int proto)
> @@ -3079,13 +3078,13 @@ static inline __wsum null_compute_pseudo(struct sk_buff *skb, int proto)
> * pseudo header.
> *
> * Return values:
> - * 0: checksum is validated or try to in skb_checksum_complete
> - * non-zero: value of invalid checksum
> + * false: checksum is validated or try to in skb_checksum_complete
> + * true: invalid checksum
> */
> #define __skb_checksum_validate(skb, proto, complete, \
> zero_okay, check, compute_pseudo) \
> ({ \
> - __sum16 __ret = 0; \
> + bool __ret = false; \
> skb->csum_valid = 0; \
> if (__skb_checksum_validate_needed(skb, zero_okay, check)) \
> __ret = __skb_checksum_validate_complete(skb, \
>
>
>
>
> --
> Sabrina
--
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
Powered by blists - more mailing lists