Subject: Re: [PATCH] net: tun: release the reference of tun device in tun_recvmsg

From: Arkadiusz Miskiewicz <[email-masked]>
Date: Wed, 29 May 2013 00:29:34 +0200

On Friday 26 of April 2013, Jason Wang wrote:
> On 04/26/2013 01:03 PM, Gao feng wrote:
> > On 04/25/2013 11:04 PM, Arkadiusz Miskiewicz wrote:
> >> On Thursday 25 of April 2013, Gao feng wrote:
> >>> We forget to release the reference of tun device in tun_recvmsg.
> >>> bug introduced in commit 54f968d6efdbf7dec36faa44fc11f01b0e4d1990
> >>> (tuntap: move socket to tun_file)
> >> 
> >> Hm, still hitting the problem even with the patch - dmesg:
> > Ok, seams there are another problems we haven't discovered.
> > it's excellent if you can find out which commit introduces this
> > bug by git bisect.
> > 
> > Thanks!
> 
> Yes, bisect will be helpful. If you could not do it, please tell me the
> steps to reproduce the issue and the tree you used. I just test with
> net.git and net-next.git with a tap device created by tunctl, didn't
> meet the issue.

Took a while. Suspected patch is below. I'm running fresh linus git
(3.10.0-rc3-00046-g58f8bbd) with the patch reverted and couldn't
reproduce problem anymore after several tries (will do more testing
in meantime).

My tap0 has ipv6 addresses (with link scope only). openvpn 2.3.1.

commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
Author: Balakumaran Kannan <[email-masked]>
Date:   Tue Apr 2 16:15:05 2013 +0530

    net IPv6 : Fix broken IPv6 routing table after loopback down-up
    
    IPv6 Routing table becomes broken once we do ifdown, ifup of the loopback(lo)
    interface. After down-up, routes of other interface's IPv6 addresses through
    'lo' are lost.
    
    IPv6 addresses assigned to all interfaces are routed through 'lo' for internal
    communication. Once 'lo' is down, those routing entries are removed from routing
    table. But those removed entries are not being re-created properly when 'lo' is
    brought up. So IPv6 addresses of other interfaces becomes unreachable from the
    same machine. Also this breaks communication with other machines because of
    NDISC packet processing failure.
    
    This patch fixes this issue by reading all interface's IPv6 addresses and adding
    them to IPv6 routing table while bringing up 'lo'.
    
    ==Testing==
    Before applying the patch:
    $ route -A inet6                                                                                                                                         
    Kernel IPv6 routing table                                                                                                                                
    Destination                    Next Hop                   Flag Met Ref Use If                                                                            
    2000::20/128                   ::                         U    256 0     0 eth0                                                                          
    fe80::/64                      ::                         U    256 0     0 eth0                                                                          
    ::/0                           ::                         !n   -1  1     1 lo                                                                            
    ::1/128                        ::                         Un   0   1     0 lo                                                                            
    2000::20/128                   ::                         Un   0   1     0 lo                                                                            
    fe80::xxxx:xxxx:xxxx:xxxx/128  ::                         Un   0   1     0 lo                                                                            
    ff00::/8                       ::                         U    256 0     0 eth0                                                                          
    ::/0                           ::                         !n   -1  1     1 lo                                                                            
    $ sudo ifdown lo
    $ sudo ifup lo
    $ route -A inet6
    Kernel IPv6 routing table
    Destination                    Next Hop                   Flag Met Ref Use If
    2000::20/128                   ::                         U    256 0     0 eth0
    fe80::/64                      ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    ::1/128                        ::                         Un   0   1     0 lo
    ff00::/8                       ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    $
    
    After applying the patch:
    $ route -A inet6
    Kernel IPv6 routing
    table
    Destination                    Next Hop                   Flag Met Ref Use If
    2000::20/128                   ::                         U    256 0     0 eth0
    fe80::/64                      ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    ::1/128                        ::                         Un   0   1     0 lo
    2000::20/128                   ::                         Un   0   1     0 lo
    fe80::xxxx:xxxx:xxxx:xxxx/128  ::                         Un   0   1     0 lo
    ff00::/8                       ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    $ sudo ifdown lo
    $ sudo ifup lo
    $ route -A inet6
    Kernel IPv6 routing table
    Destination                    Next Hop                   Flag Met Ref Use If
    2000::20/128                   ::                         U    256 0     0 eth0
    fe80::/64                      ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    ::1/128                        ::                         Un   0   1     0 lo
    2000::20/128                   ::                         Un   0   1     0 lo
    fe80::xxxx:xxxx:xxxx:xxxx/128  ::                         Un   0   1     0 lo
    ff00::/8                       ::                         U    256 0     0 eth0
    ::/0                           ::                         !n   -1  1     1 lo
    $
    
    Signed-off-by: Balakumaran Kannan <[email-masked]>
    Signed-off-by: Maruthi Thotad <[email-masked]>
    Signed-off-by: David S. Miller <[email-masked]>

git bisect start
# bad: [824282ca7d250bd7c301f221c3cd902ce906d731] Merge branch 'upstream' of git://git.linux-mips.org/pub/scm/ralf/upstream-linus
git bisect bad 824282ca7d250bd7c301f221c3cd902ce906d731
# good: [19f949f52599ba7c3f67a5897ac6be14bfcb1200] Linux 3.8
git bisect good 19f949f52599ba7c3f67a5897ac6be14bfcb1200
# good: [2ef14f465b9e096531343f5b734cffc5f759f4a6] Merge branch 'x86-mm-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 2ef14f465b9e096531343f5b734cffc5f759f4a6
# good: [21f3b24da9328415792efc780f50b9f434c12465] Merge tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi
git bisect good 21f3b24da9328415792efc780f50b9f434c12465
# skip: [b4811bacbc68f6e17d442df88f98afaa9394d4f5] ARM: fix CONFIG_VIRT_TO_BUS handling
git bisect skip b4811bacbc68f6e17d442df88f98afaa9394d4f5
# good: [cc9945bf9cac03860b2f7d59882263c965c6e3af] drm/radeon: don't set hpd, afmt interrupts when interrupts are disabled
git bisect good cc9945bf9cac03860b2f7d59882263c965c6e3af
# good: [087aa036eb79f24b856893190359ba812b460f45] powerpc: make additional room in exception vector area
git bisect good 087aa036eb79f24b856893190359ba812b460f45
# bad: [8f09aacfa6cf64c469fe60c05dfc1bd75e8615ed] Merge tag 'sound-3.9' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect bad 8f09aacfa6cf64c469fe60c05dfc1bd75e8615ed
# good: [d299c29039aae518d0e70fc26fb158a80dfcd31a] Merge tag 'for-linus-20130331' of git://git.kernel.dk/linux-block
git bisect good d299c29039aae518d0e70fc26fb158a80dfcd31a
# good: [118c9a45fdacc6fe57910fa1d048e2d5bbc193f4] Merge tag 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect good 118c9a45fdacc6fe57910fa1d048e2d5bbc193f4
# bad: [66ade474237745a57b7e87da9a93c7ec69fd52bb] Merge branch 'fixes' of git://git.linaro.org/people/rmk/linux-arm
git bisect bad 66ade474237745a57b7e87da9a93c7ec69fd52bb
# good: [6e8517a90b41b57d66926286c0add31145c75eb6] Merge tag 'regmap-v3.9-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap
git bisect good 6e8517a90b41b57d66926286c0add31145c75eb6
# skip: [cd0e4a9dd432b2f0209d84b5b2420695c1fd3eb7] Merge branch 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
git bisect skip cd0e4a9dd432b2f0209d84b5b2420695c1fd3eb7
# good: [120ecfafabec382c4feb79ff159ef42a39b6d33b] ARM: 7678/1: Work around faulty ISAR0 register in some Krait CPUs
git bisect good 120ecfafabec382c4feb79ff159ef42a39b6d33b
# bad: [17eb3d8fbe4c573426fc99946040305e79c07803] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect bad 17eb3d8fbe4c573426fc99946040305e79c07803
# skip: [765a0cac566c938821ae26efb4aa53b7502ee82c] s390/mm: provide emtpy check_pgt_cache() function
git bisect skip 765a0cac566c938821ae26efb4aa53b7502ee82c
# good: [94f9852de86447088e8e3c12d8b5a8f996acee32] s390/scm_blk: fix error return code in scm_blk_init()
git bisect good 94f9852de86447088e8e3c12d8b5a8f996acee32
# skip: [990454b5a48babde44a23c0f22bae5523f4fdf13] VSOCK: Handle changes to the VMCI context ID.
git bisect skip 990454b5a48babde44a23c0f22bae5523f4fdf13
# good: [f0f6ee1f70c4eaab9d52cf7d255df4bd89f8d1c2] cbq: incorrect processing of high limits
git bisect good f0f6ee1f70c4eaab9d52cf7d255df4bd89f8d1c2
# bad: [da241efcd9c3da2af6ba20055c7e158ec725005c] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
git bisect bad da241efcd9c3da2af6ba20055c7e158ec725005c
# skip: [25fb6ca4ed9cad72f14f61629b68dc03c0d9713f] net IPv6 : Fix broken IPv6 routing table after loopback down-up
git bisect skip 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
# only skipped commits left to test
# possible first bad commit: [da241efcd9c3da2af6ba20055c7e158ec725005c] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
# possible first bad commit: [990454b5a48babde44a23c0f22bae5523f4fdf13] VSOCK: Handle changes to the VMCI context ID.
# possible first bad commit: [25fb6ca4ed9cad72f14f61629b68dc03c0d9713f] net IPv6 : Fix broken IPv6 routing table after loopback down-up
# possible first bad commit: [94f9852de86447088e8e3c12d8b5a8f996acee32] s390/scm_blk: fix error return code in scm_blk_init()
# possible first bad commit: [3bff6038f1938d2541943dfde604a9b92f347650] s390/scm_block: fix printk format string
# possible first bad commit: [6c43a519610a5e11af5be707ad88391ce86129ba] drivers/Kconfig: add several missing GENERIC_HARDIRQS dependencies

> 
> >> http://pastebin.com/iSm0QJVY
> >> 
> >>> Signed-off-by: Gao feng <[email-masked]>
> >>> ---
> >>> 
> >>>  drivers/net/tun.c | 7 +++++--
> >>>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>> index 0c9df2f..dcd0c19 100644
> >>> --- a/drivers/net/tun.c
> >>> +++ b/drivers/net/tun.c
> >>> @@ -1471,14 +1471,17 @@ static int tun_recvmsg(struct kiocb *iocb,
> >>> struct socket *sock, if (!tun)
> >>> 
> >>>  		return -EBADFD;
> >>> 
> >>> -	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
> >>> -		return -EINVAL;
> >>> +	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
> >>> +		ret = -EINVAL;
> >>> +		goto out;
> >>> +	}
> >>> 
> >>>  	ret = tun_do_read(tun, tfile, iocb, m->msg_iov, total_len,
> >>>  	
> >>>  			  flags & MSG_DONTWAIT);
> >>>  	
> >>>  	if (ret > total_len) {
> >>>  	
> >>>  		m->msg_flags |= MSG_TRUNC;
> >>>  		ret = flags & MSG_TRUNC ? ret : total_len;
> >>>  	
> >>>  	}
> >>> 
> >>> +out:
> >>>  	tun_put(tun);
> >>>  	return ret;
> >>>  
> >>>  }

-- 
Arkadiusz Miśkiewicz, arekm / maven.pl