| 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: Thu, 05 Dec 2013 21:11:11 +0800
From: annie li <annie.li@...cle.com>
To: Paul Durrant <Paul.Durrant@...rix.com>
CC: Wei Liu <wei.liu2@...rix.com>,
Ian Campbell <Ian.Campbell@...rix.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
David Vrabel <david.vrabel@...rix.com>,
Zoltan Kiss <zoltan.kiss@...rix.com>,
David Miller <davem@...emloft.net>
Subject: Re: [Xen-devel] [PATCH net v5] xen-netback: fix fragment detection
in checksum setup
On 2013-12-5 18:45, Paul Durrant wrote:
>> -----Original Message-----
>> From: annie li [mailto:annie.li@...cle.com]
>> Sent: 05 December 2013 10:01
>> To: Paul Durrant
>> Cc: Wei Liu; Ian Campbell; netdev@...r.kernel.org; xen-devel@...ts.xen.org;
>> David Vrabel; Zoltan Kiss; David Miller
>> Subject: Re: [Xen-devel] [PATCH net v5] xen-netback: fix fragment detection
>> in checksum setup
>>
>>
>> On 2013/12/5 17:08, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: annie li [mailto:annie.li@...cle.com]
>>>> Sent: 05 December 2013 06:28
>>>> To: Paul Durrant
>>>> Cc: xen-devel@...ts.xen.org; netdev@...r.kernel.org; Zoltan Kiss; Wei
>> Liu;
>>>> Ian Campbell; David Vrabel; David Miller
>>>> Subject: Re: [PATCH net v5] xen-netback: fix fragment detection in
>> checksum
>>>> setup
>>>>
>>>>
>>>> On 2013/12/4 1:39, Paul Durrant wrote:
>>>>
>>>>> +/* This value should be large enough to cover a tagged ethernet
>> header
>>>> plus
>>>>> + * an IPv6 header, all options, and a maximal TCP or UDP header.
>>>>> + */
>>>>> +#define MAX_IPV6_HDR_LEN 256
>>>>> +
>>>>> +#define OPT_HDR(type, skb, off) \
>>>>> + (type *)(skb_network_header(skb) + (off))
>>>>> +
>>>>> static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
>>>>> int recalculate_partial_csum)
>>>>> {
>>>>> - int err = -EPROTO;
>>>>> - struct ipv6hdr *ipv6h = (void *)skb->data;
>>>>> + int err;
>>>>> u8 nexthdr;
>>>>> - unsigned int header_size;
>>>>> unsigned int off;
>>>>> + unsigned int len;
>>>>> bool fragment;
>>>>> bool done;
>>>>>
>>>>> + fragment = false;
>>>>> done = false;
>>>>>
>>>>> off = sizeof(struct ipv6hdr);
>>>>>
>>>>> - header_size = skb->network_header + off;
>>>>> - maybe_pull_tail(skb, header_size);
>>>>> + err = maybe_pull_tail(skb, off, MAX_IPV6_HDR_LEN);
>>>>> + if (err < 0)
>>>>> + goto out;
>>>>>
>>>>> - nexthdr = ipv6h->nexthdr;
>>>>> + nexthdr = ipv6_hdr(skb)->nexthdr;
>>>>>
>>>>> - while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len))
>>>> &&
>>>>> - !done) {
>>>>> + len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
>>>>> + while (off <= len && !done) {
>>>>> switch (nexthdr) {
>>>>> case IPPROTO_DSTOPTS:
>>>>> case IPPROTO_HOPOPTS:
>>>>> case IPPROTO_ROUTING: {
>>>>> - struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
>>>>> + struct ipv6_opt_hdr *hp;
>>>>>
>>>>> - header_size = skb->network_header +
>>>>> - off +
>>>>> - sizeof(struct ipv6_opt_hdr);
>>>>> - maybe_pull_tail(skb, header_size);
>>>>> + err = maybe_pull_tail(skb,
>>>>> + off +
>>>>> + sizeof(struct ipv6_opt_hdr),
>>>>> + MAX_IPV6_HDR_LEN);
>>>>> + if (err < 0)
>>>>> + goto out;
>>>>>
>>>>> + hp = OPT_HDR(struct ipv6_opt_hdr, skb, off);
>>>>> nexthdr = hp->nexthdr;
>>>>> off += ipv6_optlen(hp);
>>>>> break;
>>>>> }
>>>>> case IPPROTO_AH: {
>>>>> - struct ip_auth_hdr *hp = (void *)(skb->data + off);
>>>>> + struct ip_auth_hdr *hp;
>>>>> +
>>>>> + err = maybe_pull_tail(skb,
>>>>> + off +
>>>>> + sizeof(struct ip_auth_hdr),
>>>>> + MAX_IPV6_HDR_LEN);
>>>>> + if (err < 0)
>>>>> + goto out;
>>>>> +
>>>>> + hp = OPT_HDR(struct ip_auth_hdr, skb, off);
>>>>> + nexthdr = hp->nexthdr;
>>>>> + off += ipv6_authlen(hp);
>>>>> + break;
>>>>> + }
>>>>> + case IPPROTO_FRAGMENT: {
>>>>> + struct frag_hdr *hp;
>>>>>
>>>>> - header_size = skb->network_header +
>>>>> - off +
>>>>> - sizeof(struct ip_auth_hdr);
>>>>> - maybe_pull_tail(skb, header_size);
>>>>> + err = maybe_pull_tail(skb,
>>>>> + off +
>>>>> + sizeof(struct frag_hdr),
>>>>> + MAX_IPV6_HDR_LEN);
>>>>> + if (err < 0)
>>>>> + goto out;
>>>>> +
>>>>> + hp = OPT_HDR(struct frag_hdr, skb, off);
>>>>> +
>>>>> + if (hp->frag_off & htons(IP6_OFFSET | IP6_MF))
>>>>> + fragment = true;
>>>>>
>>>>> nexthdr = hp->nexthdr;
>>>>> - off += (hp->hdrlen+2)<<2;
>>>>> + off += sizeof(struct frag_hdr);
>>>>> break;
>>>>> }
>>>>> - case IPPROTO_FRAGMENT:
>>>>> - fragment = true;
>>>>> - /* fall through */
>>>> About IPv6 extension headers, should "Encapsulating Security Payload"
>> be
>>>> processed too?(See rfc2460 and rfc2406) Is there any concern to ignore
>>>> it here?
>>>>
>>> It's deliberately ignored. If we have an ESP header then the TCP/UDP
>> header and payload will be encrypted so you can't do checksum offload,
>> hence this function should return an error.
>> Got it. But from current code, if ESP extension header exists, switch
>> (nexthdr) will go into default case directly, and done is set with true,
>> then the code quits switch and while loop.
> That's correct. ESP is considered a terminating header.
>
>> After that, it will go into
>> next switch and then the default case since the nexthdr is ipv6 ESP
>> extension header, not TCP/UDP header. Then netdev_err probably prints
>> out improper log.
>>
> Which netdev_err? This patch should remove all calls to netdev_err from this codepath. Did I miss one?
You are right! I mixed up this patch and original source code.
Thanks
Annie
--
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