| 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, 5 Dec 2013 10:45:48 +0000
From: Paul Durrant <Paul.Durrant@...rix.com>
To: annie li <annie.li@...cle.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
> -----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?
Paul
--
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