diff mbox series

[v3,net] ipv6: Fix dangling pointer when ipv6 fragment

Message ID 44e8dcf8-bf64-0407-65bb-122d0853c672@huawei.com
State Changes Requested
Delegated to: David Miller
Headers show
Series [v3,net] ipv6: Fix dangling pointer when ipv6 fragment | expand

Commit Message

hujunwei April 2, 2019, 3:09 a.m. UTC
From: Junwei Hu <hujunwei4@huawei.com>

At the beginning of ip6_fragment func, the prevhdr pointer is
obtained in the ip6_find_1stfragopt func.
However, all the pointers pointing into skb header may change
when calling skb_checksum_help func with
skb->ip_summed = CHECKSUM_PARTIAL condition.
The prevhdr pointe will be dangling if it is not reloaded after
calling __skb_linearize func in skb_checksum_help func.

Here, I add a variable, nexthdr_offset, to evaluate the offset,
which does not changes even after calling __skb_linearize func.

Fixes: 405c92f7a541 ("ipv6: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment")
Signed-off-by: Junwei Hu <hujunwei4@huawei.com>
Reported-by: Wenhao Zhang <zhangwenhao8@huawei.com>
Reported-by: syzbot+e8ce541d095e486074fc@syzkaller.appspotmail.com
Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
V2->V3:
- fix patch format issue
- add Acked-by

 net/ipv6/ip6_output.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

kernel test robot April 2, 2019, 10:49 a.m. UTC | #1
Hi hujunwei,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/hujunwei/ipv6-Fix-dangling-pointer-when-ipv6-fragment/20190402-175602
config: i386-randconfig-x005-201913 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   net//ipv6/ip6_output.c: In function 'ip6_fragment':
>> net//ipv6/ip6_output.c:609:27: warning: 'prevhdr' is used uninitialized in this function [-Wuninitialized]
     nexthdr_offset = prevhdr - skb_network_header(skb);
                      ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~

vim +/prevhdr +609 net//ipv6/ip6_output.c

   594	
   595	int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
   596			 int (*output)(struct net *, struct sock *, struct sk_buff *))
   597	{
   598		struct sk_buff *frag;
   599		struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
   600		struct ipv6_pinfo *np = skb->sk && !dev_recursion_level() ?
   601					inet6_sk(skb->sk) : NULL;
   602		struct ipv6hdr *tmp_hdr;
   603		struct frag_hdr *fh;
   604		unsigned int mtu, hlen, left, len, nexthdr_offset;
   605		int hroom, troom;
   606		__be32 frag_id;
   607		int ptr, offset = 0, err = 0;
   608		u8 *prevhdr, nexthdr = 0;
 > 609		nexthdr_offset = prevhdr - skb_network_header(skb);
   610	
   611		err = ip6_find_1stfragopt(skb, &prevhdr);
   612		if (err < 0)
   613			goto fail;
   614		hlen = err;
   615		nexthdr = *prevhdr;
   616	
   617		mtu = ip6_skb_dst_mtu(skb);
   618	
   619		/* We must not fragment if the socket is set to force MTU discovery
   620		 * or if the skb it not generated by a local socket.
   621		 */
   622		if (unlikely(!skb->ignore_df && skb->len > mtu))
   623			goto fail_toobig;
   624	
   625		if (IP6CB(skb)->frag_max_size) {
   626			if (IP6CB(skb)->frag_max_size > mtu)
   627				goto fail_toobig;
   628	
   629			/* don't send fragments larger than what we received */
   630			mtu = IP6CB(skb)->frag_max_size;
   631			if (mtu < IPV6_MIN_MTU)
   632				mtu = IPV6_MIN_MTU;
   633		}
   634	
   635		if (np && np->frag_size < mtu) {
   636			if (np->frag_size)
   637				mtu = np->frag_size;
   638		}
   639		if (mtu < hlen + sizeof(struct frag_hdr) + 8)
   640			goto fail_toobig;
   641		mtu -= hlen + sizeof(struct frag_hdr);
   642	
   643		frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
   644					    &ipv6_hdr(skb)->saddr);
   645	
   646		if (skb->ip_summed == CHECKSUM_PARTIAL &&
   647		    (err = skb_checksum_help(skb)))
   648			goto fail;
   649	
   650		prevhdr = skb_network_header(skb) + nexthdr_offset;
   651		hroom = LL_RESERVED_SPACE(rt->dst.dev);
   652		if (skb_has_frag_list(skb)) {
   653			unsigned int first_len = skb_pagelen(skb);
   654			struct sk_buff *frag2;
   655	
   656			if (first_len - hlen > mtu ||
   657			    ((first_len - hlen) & 7) ||
   658			    skb_cloned(skb) ||
   659			    skb_headroom(skb) < (hroom + sizeof(struct frag_hdr)))
   660				goto slow_path;
   661	
   662			skb_walk_frags(skb, frag) {
   663				/* Correct geometry. */
   664				if (frag->len > mtu ||
   665				    ((frag->len & 7) && frag->next) ||
   666				    skb_headroom(frag) < (hlen + hroom + sizeof(struct frag_hdr)))
   667					goto slow_path_clean;
   668	
   669				/* Partially cloned skb? */
   670				if (skb_shared(frag))
   671					goto slow_path_clean;
   672	
   673				BUG_ON(frag->sk);
   674				if (skb->sk) {
   675					frag->sk = skb->sk;
   676					frag->destructor = sock_wfree;
   677				}
   678				skb->truesize -= frag->truesize;
   679			}
   680	
   681			err = 0;
   682			offset = 0;
   683			/* BUILD HEADER */
   684	
   685			*prevhdr = NEXTHDR_FRAGMENT;
   686			tmp_hdr = kmemdup(skb_network_header(skb), hlen, GFP_ATOMIC);
   687			if (!tmp_hdr) {
   688				err = -ENOMEM;
   689				goto fail;
   690			}
   691			frag = skb_shinfo(skb)->frag_list;
   692			skb_frag_list_init(skb);
   693	
   694			__skb_pull(skb, hlen);
   695			fh = __skb_push(skb, sizeof(struct frag_hdr));
   696			__skb_push(skb, hlen);
   697			skb_reset_network_header(skb);
   698			memcpy(skb_network_header(skb), tmp_hdr, hlen);
   699	
   700			fh->nexthdr = nexthdr;
   701			fh->reserved = 0;
   702			fh->frag_off = htons(IP6_MF);
   703			fh->identification = frag_id;
   704	
   705			first_len = skb_pagelen(skb);
   706			skb->data_len = first_len - skb_headlen(skb);
   707			skb->len = first_len;
   708			ipv6_hdr(skb)->payload_len = htons(first_len -
   709							   sizeof(struct ipv6hdr));
   710	
   711			for (;;) {
   712				/* Prepare header of the next frame,
   713				 * before previous one went down. */
   714				if (frag) {
   715					frag->ip_summed = CHECKSUM_NONE;
   716					skb_reset_transport_header(frag);
   717					fh = __skb_push(frag, sizeof(struct frag_hdr));
   718					__skb_push(frag, hlen);
   719					skb_reset_network_header(frag);
   720					memcpy(skb_network_header(frag), tmp_hdr,
   721					       hlen);
   722					offset += skb->len - hlen - sizeof(struct frag_hdr);
   723					fh->nexthdr = nexthdr;
   724					fh->reserved = 0;
   725					fh->frag_off = htons(offset);
   726					if (frag->next)
   727						fh->frag_off |= htons(IP6_MF);
   728					fh->identification = frag_id;
   729					ipv6_hdr(frag)->payload_len =
   730							htons(frag->len -
   731							      sizeof(struct ipv6hdr));
   732					ip6_copy_metadata(frag, skb);
   733				}
   734	
   735				err = output(net, sk, skb);
   736				if (!err)
   737					IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
   738						      IPSTATS_MIB_FRAGCREATES);
   739	
   740				if (err || !frag)
   741					break;
   742	
   743				skb = frag;
   744				frag = skb->next;
   745				skb_mark_not_on_list(skb);
   746			}
   747	
   748			kfree(tmp_hdr);
   749	
   750			if (err == 0) {
   751				IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
   752					      IPSTATS_MIB_FRAGOKS);
   753				return 0;
   754			}
   755	
   756			kfree_skb_list(frag);
   757	
   758			IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
   759				      IPSTATS_MIB_FRAGFAILS);
   760			return err;
   761	
   762	slow_path_clean:
   763			skb_walk_frags(skb, frag2) {
   764				if (frag2 == frag)
   765					break;
   766				frag2->sk = NULL;
   767				frag2->destructor = NULL;
   768				skb->truesize += frag2->truesize;
   769			}
   770		}
   771	
   772	slow_path:
   773		left = skb->len - hlen;		/* Space per frame */
   774		ptr = hlen;			/* Where to start from */
   775	
   776		/*
   777		 *	Fragment the datagram.
   778		 */
   779	
   780		troom = rt->dst.dev->needed_tailroom;
   781	
   782		/*
   783		 *	Keep copying data until we run out.
   784		 */
   785		while (left > 0)	{
   786			u8 *fragnexthdr_offset;
   787	
   788			len = left;
   789			/* IF: it doesn't fit, use 'mtu' - the data space left */
   790			if (len > mtu)
   791				len = mtu;
   792			/* IF: we are not sending up to and including the packet end
   793			   then align the next start on an eight byte boundary */
   794			if (len < left)	{
   795				len &= ~7;
   796			}
   797	
   798			/* Allocate buffer */
   799			frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) +
   800					 hroom + troom, GFP_ATOMIC);
   801			if (!frag) {
   802				err = -ENOMEM;
   803				goto fail;
   804			}
   805	
   806			/*
   807			 *	Set up data on packet
   808			 */
   809	
   810			ip6_copy_metadata(frag, skb);
   811			skb_reserve(frag, hroom);
   812			skb_put(frag, len + hlen + sizeof(struct frag_hdr));
   813			skb_reset_network_header(frag);
   814			fh = (struct frag_hdr *)(skb_network_header(frag) + hlen);
   815			frag->transport_header = (frag->network_header + hlen +
   816						  sizeof(struct frag_hdr));
   817	
   818			/*
   819			 *	Charge the memory for the fragment to any owner
   820			 *	it might possess
   821			 */
   822			if (skb->sk)
   823				skb_set_owner_w(frag, skb->sk);
   824	
   825			/*
   826			 *	Copy the packet header into the new buffer.
   827			 */
   828			skb_copy_from_linear_data(skb, skb_network_header(frag), hlen);
   829	
   830			fragnexthdr_offset = skb_network_header(frag);
   831			fragnexthdr_offset += prevhdr - skb_network_header(skb);
   832			*fragnexthdr_offset = NEXTHDR_FRAGMENT;
   833	
   834			/*
   835			 *	Build fragment header.
   836			 */
   837			fh->nexthdr = nexthdr;
   838			fh->reserved = 0;
   839			fh->identification = frag_id;
   840	
   841			/*
   842			 *	Copy a block of the IP datagram.
   843			 */
   844			BUG_ON(skb_copy_bits(skb, ptr, skb_transport_header(frag),
   845					     len));
   846			left -= len;
   847	
   848			fh->frag_off = htons(offset);
   849			if (left > 0)
   850				fh->frag_off |= htons(IP6_MF);
   851			ipv6_hdr(frag)->payload_len = htons(frag->len -
   852							    sizeof(struct ipv6hdr));
   853	
   854			ptr += len;
   855			offset += len;
   856	
   857			/*
   858			 *	Put this fragment into the sending queue.
   859			 */
   860			err = output(net, sk, frag);
   861			if (err)
   862				goto fail;
   863	
   864			IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
   865				      IPSTATS_MIB_FRAGCREATES);
   866		}
   867		IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
   868			      IPSTATS_MIB_FRAGOKS);
   869		consume_skb(skb);
   870		return err;
   871	
   872	fail_toobig:
   873		if (skb->sk && dst_allfrag(skb_dst(skb)))
   874			sk_nocaps_add(skb->sk, NETIF_F_GSO_MASK);
   875	
   876		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
   877		err = -EMSGSIZE;
   878	
   879	fail:
   880		IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
   881			      IPSTATS_MIB_FRAGFAILS);
   882		kfree_skb(skb);
   883		return err;
   884	}
   885	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Martin KaFai Lau April 2, 2019, 3:34 p.m. UTC | #2
On Tue, Apr 02, 2019 at 06:49:03PM +0800, kbuild test robot wrote:
> Hi hujunwei,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on net/master]
> 
> url:    https://github.com/0day-ci/linux/commits/hujunwei/ipv6-Fix-dangling-pointer-when-ipv6-fragment/20190402-175602
> config: i386-randconfig-x005-201913 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>    net//ipv6/ip6_output.c: In function 'ip6_fragment':
> >> net//ipv6/ip6_output.c:609:27: warning: 'prevhdr' is used uninitialized in this function [-Wuninitialized]
>      nexthdr_offset = prevhdr - skb_network_header(skb);
>                       ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> vim +/prevhdr +609 net//ipv6/ip6_output.c
> 
>    594	
>    595	int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
>    596			 int (*output)(struct net *, struct sock *, struct sk_buff *))
>    597	{
>    598		struct sk_buff *frag;
>    599		struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
>    600		struct ipv6_pinfo *np = skb->sk && !dev_recursion_level() ?
>    601					inet6_sk(skb->sk) : NULL;
>    602		struct ipv6hdr *tmp_hdr;
>    603		struct frag_hdr *fh;
>    604		unsigned int mtu, hlen, left, len, nexthdr_offset;
>    605		int hroom, troom;
>    606		__be32 frag_id;
>    607		int ptr, offset = 0, err = 0;
>    608		u8 *prevhdr, nexthdr = 0;
>  > 609		nexthdr_offset = prevhdr - skb_network_header(skb);
hmm... This line has been moved up since v2. :(

>    610	
>    611		err = ip6_find_1stfragopt(skb, &prevhdr);
>    612		if (err < 0)
>    613			goto fail;
>    614		hlen = err;
>    615		nexthdr = *prevhdr;
>    616	
>    617		mtu = ip6_skb_dst_mtu(skb);
>    618	
>    619		/* We must not fragment if the socket is set to force MTU discovery
>    620		 * or if the skb it not generated by a local socket.
>    621		 */
>    622		if (unlikely(!skb->ignore_df && skb->len > mtu))
>    623			goto fail_toobig;
>    624	
>    625		if (IP6CB(skb)->frag_max_size) {
>    626			if (IP6CB(skb)->frag_max_size > mtu)
>    627				goto fail_toobig;
>    628	
>    629			/* don't send fragments larger than what we received */
>    630			mtu = IP6CB(skb)->frag_max_size;
>    631			if (mtu < IPV6_MIN_MTU)
>    632				mtu = IPV6_MIN_MTU;
>    633		}
>    634	
>    635		if (np && np->frag_size < mtu) {
>    636			if (np->frag_size)
>    637				mtu = np->frag_size;
>    638		}
>    639		if (mtu < hlen + sizeof(struct frag_hdr) + 8)
>    640			goto fail_toobig;
>    641		mtu -= hlen + sizeof(struct frag_hdr);
>    642	
>    643		frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
>    644					    &ipv6_hdr(skb)->saddr);
>    645	
>    646		if (skb->ip_summed == CHECKSUM_PARTIAL &&
>    647		    (err = skb_checksum_help(skb)))
>    648			goto fail;
>    649	
>    650		prevhdr = skb_network_header(skb) + nexthdr_offset;
>    651		hroom = LL_RESERVED_SPACE(rt->dst.dev);
>    652		if (skb_has_frag_list(skb)) {
>    653			unsigned int first_len = skb_pagelen(skb);
>    654			struct sk_buff *frag2;
>    655	
>    656			if (first_len - hlen > mtu ||
>    657			    ((first_len - hlen) & 7) ||
>    658			    skb_cloned(skb) ||
>    659			    skb_headroom(skb) < (hroom + sizeof(struct frag_hdr)))
>    660				goto slow_path;
>    661	
>    662			skb_walk_frags(skb, frag) {
>    663				/* Correct geometry. */
>    664				if (frag->len > mtu ||
>    665				    ((frag->len & 7) && frag->next) ||
>    666				    skb_headroom(frag) < (hlen + hroom + sizeof(struct frag_hdr)))
>    667					goto slow_path_clean;
>    668	
>    669				/* Partially cloned skb? */
>    670				if (skb_shared(frag))
>    671					goto slow_path_clean;
>    672	
>    673				BUG_ON(frag->sk);
>    674				if (skb->sk) {
>    675					frag->sk = skb->sk;
>    676					frag->destructor = sock_wfree;
>    677				}
>    678				skb->truesize -= frag->truesize;
>    679			}
>    680	
>    681			err = 0;
>    682			offset = 0;
>    683			/* BUILD HEADER */
>    684	
>    685			*prevhdr = NEXTHDR_FRAGMENT;
>    686			tmp_hdr = kmemdup(skb_network_header(skb), hlen, GFP_ATOMIC);
>    687			if (!tmp_hdr) {
>    688				err = -ENOMEM;
>    689				goto fail;
>    690			}
>    691			frag = skb_shinfo(skb)->frag_list;
>    692			skb_frag_list_init(skb);
>    693	
>    694			__skb_pull(skb, hlen);
>    695			fh = __skb_push(skb, sizeof(struct frag_hdr));
>    696			__skb_push(skb, hlen);
>    697			skb_reset_network_header(skb);
>    698			memcpy(skb_network_header(skb), tmp_hdr, hlen);
>    699	
>    700			fh->nexthdr = nexthdr;
>    701			fh->reserved = 0;
>    702			fh->frag_off = htons(IP6_MF);
>    703			fh->identification = frag_id;
>    704	
>    705			first_len = skb_pagelen(skb);
>    706			skb->data_len = first_len - skb_headlen(skb);
>    707			skb->len = first_len;
>    708			ipv6_hdr(skb)->payload_len = htons(first_len -
>    709							   sizeof(struct ipv6hdr));
>    710	
>    711			for (;;) {
>    712				/* Prepare header of the next frame,
>    713				 * before previous one went down. */
>    714				if (frag) {
>    715					frag->ip_summed = CHECKSUM_NONE;
>    716					skb_reset_transport_header(frag);
>    717					fh = __skb_push(frag, sizeof(struct frag_hdr));
>    718					__skb_push(frag, hlen);
>    719					skb_reset_network_header(frag);
>    720					memcpy(skb_network_header(frag), tmp_hdr,
>    721					       hlen);
>    722					offset += skb->len - hlen - sizeof(struct frag_hdr);
>    723					fh->nexthdr = nexthdr;
>    724					fh->reserved = 0;
>    725					fh->frag_off = htons(offset);
>    726					if (frag->next)
>    727						fh->frag_off |= htons(IP6_MF);
>    728					fh->identification = frag_id;
>    729					ipv6_hdr(frag)->payload_len =
>    730							htons(frag->len -
>    731							      sizeof(struct ipv6hdr));
>    732					ip6_copy_metadata(frag, skb);
>    733				}
>    734	
>    735				err = output(net, sk, skb);
>    736				if (!err)
>    737					IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
>    738						      IPSTATS_MIB_FRAGCREATES);
>    739	
>    740				if (err || !frag)
>    741					break;
>    742	
>    743				skb = frag;
>    744				frag = skb->next;
>    745				skb_mark_not_on_list(skb);
>    746			}
>    747	
>    748			kfree(tmp_hdr);
>    749	
>    750			if (err == 0) {
>    751				IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
>    752					      IPSTATS_MIB_FRAGOKS);
>    753				return 0;
>    754			}
>    755	
>    756			kfree_skb_list(frag);
>    757	
>    758			IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
>    759				      IPSTATS_MIB_FRAGFAILS);
>    760			return err;
>    761	
>    762	slow_path_clean:
>    763			skb_walk_frags(skb, frag2) {
>    764				if (frag2 == frag)
>    765					break;
>    766				frag2->sk = NULL;
>    767				frag2->destructor = NULL;
>    768				skb->truesize += frag2->truesize;
>    769			}
>    770		}
>    771	
>    772	slow_path:
>    773		left = skb->len - hlen;		/* Space per frame */
>    774		ptr = hlen;			/* Where to start from */
>    775	
>    776		/*
>    777		 *	Fragment the datagram.
>    778		 */
>    779	
>    780		troom = rt->dst.dev->needed_tailroom;
>    781	
>    782		/*
>    783		 *	Keep copying data until we run out.
>    784		 */
>    785		while (left > 0)	{
>    786			u8 *fragnexthdr_offset;
>    787	
>    788			len = left;
>    789			/* IF: it doesn't fit, use 'mtu' - the data space left */
>    790			if (len > mtu)
>    791				len = mtu;
>    792			/* IF: we are not sending up to and including the packet end
>    793			   then align the next start on an eight byte boundary */
>    794			if (len < left)	{
>    795				len &= ~7;
>    796			}
>    797	
>    798			/* Allocate buffer */
>    799			frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) +
>    800					 hroom + troom, GFP_ATOMIC);
>    801			if (!frag) {
>    802				err = -ENOMEM;
>    803				goto fail;
>    804			}
>    805	
>    806			/*
>    807			 *	Set up data on packet
>    808			 */
>    809	
>    810			ip6_copy_metadata(frag, skb);
>    811			skb_reserve(frag, hroom);
>    812			skb_put(frag, len + hlen + sizeof(struct frag_hdr));
>    813			skb_reset_network_header(frag);
>    814			fh = (struct frag_hdr *)(skb_network_header(frag) + hlen);
>    815			frag->transport_header = (frag->network_header + hlen +
>    816						  sizeof(struct frag_hdr));
>    817	
>    818			/*
>    819			 *	Charge the memory for the fragment to any owner
>    820			 *	it might possess
>    821			 */
>    822			if (skb->sk)
>    823				skb_set_owner_w(frag, skb->sk);
>    824	
>    825			/*
>    826			 *	Copy the packet header into the new buffer.
>    827			 */
>    828			skb_copy_from_linear_data(skb, skb_network_header(frag), hlen);
>    829	
>    830			fragnexthdr_offset = skb_network_header(frag);
>    831			fragnexthdr_offset += prevhdr - skb_network_header(skb);
>    832			*fragnexthdr_offset = NEXTHDR_FRAGMENT;
>    833	
>    834			/*
>    835			 *	Build fragment header.
>    836			 */
>    837			fh->nexthdr = nexthdr;
>    838			fh->reserved = 0;
>    839			fh->identification = frag_id;
>    840	
>    841			/*
>    842			 *	Copy a block of the IP datagram.
>    843			 */
>    844			BUG_ON(skb_copy_bits(skb, ptr, skb_transport_header(frag),
>    845					     len));
>    846			left -= len;
>    847	
>    848			fh->frag_off = htons(offset);
>    849			if (left > 0)
>    850				fh->frag_off |= htons(IP6_MF);
>    851			ipv6_hdr(frag)->payload_len = htons(frag->len -
>    852							    sizeof(struct ipv6hdr));
>    853	
>    854			ptr += len;
>    855			offset += len;
>    856	
>    857			/*
>    858			 *	Put this fragment into the sending queue.
>    859			 */
>    860			err = output(net, sk, frag);
>    861			if (err)
>    862				goto fail;
>    863	
>    864			IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
>    865				      IPSTATS_MIB_FRAGCREATES);
>    866		}
>    867		IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
>    868			      IPSTATS_MIB_FRAGOKS);
>    869		consume_skb(skb);
>    870		return err;
>    871	
>    872	fail_toobig:
>    873		if (skb->sk && dst_allfrag(skb_dst(skb)))
>    874			sk_nocaps_add(skb->sk, NETIF_F_GSO_MASK);
>    875	
>    876		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
>    877		err = -EMSGSIZE;
>    878	
>    879	fail:
>    880		IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
>    881			      IPSTATS_MIB_FRAGFAILS);
>    882		kfree_skb(skb);
>    883		return err;
>    884	}
>    885	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_pipermail_kbuild-2Dall&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=VM2d9MD3GON8eQRY0bYRU7OGyFCoSiaFiYJJa6-3rDU&s=p3p0GnDqN_d4cCUIgH993sIXi_Sw8tUInnni3uMPXKk&e=                   Intel Corporation
hujunwei April 3, 2019, 1:32 a.m. UTC | #3
On 2019/4/2 23:34, Martin Lau wrote:
> On Tue, Apr 02, 2019 at 06:49:03PM +0800, kbuild test robot wrote:
>> Hi hujunwei,
>>
>> Thank you for the patch! Perhaps something to improve:
>>
>> [auto build test WARNING on net/master]
>>
>>
>> vim +/prevhdr +609 net//ipv6/ip6_output.c
>>
>>    594	
>>    595	int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
>>    596			 int (*output)(struct net *, struct sock *, struct sk_buff *))
>>    597	{
>>    598		struct sk_buff *frag;
>>    599		struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
>>    600		struct ipv6_pinfo *np = skb->sk && !dev_recursion_level() ?
>>    601					inet6_sk(skb->sk) : NULL;
>>    602		struct ipv6hdr *tmp_hdr;
>>    603		struct frag_hdr *fh;
>>    604		unsigned int mtu, hlen, left, len, nexthdr_offset;
>>    605		int hroom, troom;
>>    606		__be32 frag_id;
>>    607		int ptr, offset = 0, err = 0;
>>    608		u8 *prevhdr, nexthdr = 0;
>>  > 609		nexthdr_offset = prevhdr - skb_network_header(skb);
> hmm... This line has been moved up since v2. :(

Hi Martin,
Thank you for your remind, I sorry for this, i send the patch v4 yesterday.
Dan Carpenter April 3, 2019, 5:01 a.m. UTC | #4
Hi hujunwei,

url:    https://github.com/0day-ci/linux/commits/hujunwei/ipv6-Fix-dangling-pointer-when-ipv6-fragment/20190402-175602

New smatch warnings:
net/ipv6/ip6_output.c:609 ip6_fragment() error: uninitialized symbol 'prevhdr'.

Old smatch warnings:
net/ipv6/ip6_output.c:247 ip6_xmit() error: we previously assumed 'np' could be null (see line 241)

# https://github.com/0day-ci/linux/commit/7f25fe5b3011737e52e4d8b4a2dfcafd46677115
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 7f25fe5b3011737e52e4d8b4a2dfcafd46677115
vim +/prevhdr +609 net/ipv6/ip6_output.c

^1da177e4 Linus Torvalds             2005-04-16  594  
7d8c6e391 Eric W. Biederman          2015-06-12  595  int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
7d8c6e391 Eric W. Biederman          2015-06-12  596  		 int (*output)(struct net *, struct sock *, struct sk_buff *))
^1da177e4 Linus Torvalds             2005-04-16  597  {
^1da177e4 Linus Torvalds             2005-04-16  598  	struct sk_buff *frag;
adf30907d Eric Dumazet               2009-06-02  599  	struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
f60e5990d hannes@stressinduktion.org 2015-04-01  600  	struct ipv6_pinfo *np = skb->sk && !dev_recursion_level() ?
f60e5990d hannes@stressinduktion.org 2015-04-01  601  				inet6_sk(skb->sk) : NULL;
^1da177e4 Linus Torvalds             2005-04-16  602  	struct ipv6hdr *tmp_hdr;
^1da177e4 Linus Torvalds             2005-04-16  603  	struct frag_hdr *fh;
7f25fe5b3 Junwei Hu                  2019-04-02  604  	unsigned int mtu, hlen, left, len, nexthdr_offset;
a7ae19922 Herbert Xu                 2011-11-18  605  	int hroom, troom;
286c2349f Martin KaFai Lau           2015-05-22  606  	__be32 frag_id;
^1da177e4 Linus Torvalds             2005-04-16  607  	int ptr, offset = 0, err = 0;
^1da177e4 Linus Torvalds             2005-04-16  608  	u8 *prevhdr, nexthdr = 0;
                                                            ^^^^^^^^
7f25fe5b3 Junwei Hu                  2019-04-02 @609  	nexthdr_offset = prevhdr - skb_network_header(skb);
                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
^1da177e4 Linus Torvalds             2005-04-16  610  
7dd7eb951 David S. Miller            2017-05-17  611  	err = ip6_find_1stfragopt(skb, &prevhdr);
                                                                                       ^^^^^^^^
7dd7eb951 David S. Miller            2017-05-17  612  	if (err < 0)
2423496af Craig Gallek               2017-05-16  613  		goto fail;
7dd7eb951 David S. Miller            2017-05-17  614  	hlen = err;
^1da177e4 Linus Torvalds             2005-04-16  615  	nexthdr = *prevhdr;
^1da177e4 Linus Torvalds             2005-04-16  616  
628a5c561 John Heffner               2007-04-20  617  	mtu = ip6_skb_dst_mtu(skb);
b881ef760 John Heffner               2007-04-20  618  
b881ef760 John Heffner               2007-04-20  619  	/* We must not fragment if the socket is set to force MTU discovery

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
hujunwei April 3, 2019, 6:47 a.m. UTC | #5
On 2019/4/3 13:01, Dan Carpenter wrote:
> Hi hujunwei,
> 
> url:    https://github.com/0day-ci/linux/commits/hujunwei/ipv6-Fix-dangling-pointer-when-ipv6-fragment/20190402-175602
> 
> New smatch warnings:
> net/ipv6/ip6_output.c:609 ip6_fragment() error: uninitialized symbol 'prevhdr'.
> 
> Old smatch warnings:
> net/ipv6/ip6_output.c:247 ip6_xmit() error: we previously assumed 'np' could be null (see line 241)
> 
> # https://github.com/0day-ci/linux/commit/7f25fe5b3011737e52e4d8b4a2dfcafd46677115
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 7f25fe5b3011737e52e4d8b4a2dfcafd46677115
> vim +/prevhdr +609 net/ipv6/ip6_output.c
> 
> ^1da177e4 Linus Torvalds             2005-04-16  594  
> 7d8c6e391 Eric W. Biederman          2015-06-12  595  int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
> 7d8c6e391 Eric W. Biederman          2015-06-12  596  		 int (*output)(struct net *, struct sock *, struct sk_buff *))
> ^1da177e4 Linus Torvalds             2005-04-16  597  {
> ^1da177e4 Linus Torvalds             2005-04-16  598  	struct sk_buff *frag;
> adf30907d Eric Dumazet               2009-06-02  599  	struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
> f60e5990d hannes@stressinduktion.org 2015-04-01  600  	struct ipv6_pinfo *np = skb->sk && !dev_recursion_level() ?
> f60e5990d hannes@stressinduktion.org 2015-04-01  601  				inet6_sk(skb->sk) : NULL;
> ^1da177e4 Linus Torvalds             2005-04-16  602  	struct ipv6hdr *tmp_hdr;
> ^1da177e4 Linus Torvalds             2005-04-16  603  	struct frag_hdr *fh;
> 7f25fe5b3 Junwei Hu                  2019-04-02  604  	unsigned int mtu, hlen, left, len, nexthdr_offset;
> a7ae19922 Herbert Xu                 2011-11-18  605  	int hroom, troom;
> 286c2349f Martin KaFai Lau           2015-05-22  606  	__be32 frag_id;
> ^1da177e4 Linus Torvalds             2005-04-16  607  	int ptr, offset = 0, err = 0;
> ^1da177e4 Linus Torvalds             2005-04-16  608  	u8 *prevhdr, nexthdr = 0;
>                                                             ^^^^^^^^
> 7f25fe5b3 Junwei Hu                  2019-04-02 @609  	nexthdr_offset = prevhdr - skb_network_header(skb);
>                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 
> .
> 
Hi Dan,
Thank you for your remind, I sorry for this, i send the patch v4 yesterday. You can get it from the mail list.
diff mbox series

Patch

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index edbd12067170..533be3268e52 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -601,11 +601,12 @@  int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 				inet6_sk(skb->sk) : NULL;
 	struct ipv6hdr *tmp_hdr;
 	struct frag_hdr *fh;
-	unsigned int mtu, hlen, left, len;
+	unsigned int mtu, hlen, left, len, nexthdr_offset;
 	int hroom, troom;
 	__be32 frag_id;
 	int ptr, offset = 0, err = 0;
 	u8 *prevhdr, nexthdr = 0;
+	nexthdr_offset = prevhdr - skb_network_header(skb);

 	err = ip6_find_1stfragopt(skb, &prevhdr);
 	if (err < 0)
@@ -646,6 +647,7 @@  int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 	    (err = skb_checksum_help(skb)))
 		goto fail;

+	prevhdr = skb_network_header(skb) + nexthdr_offset;
 	hroom = LL_RESERVED_SPACE(rt->dst.dev);
 	if (skb_has_frag_list(skb)) {
 		unsigned int first_len = skb_pagelen(skb);