Regarding _skb_refdst memory alloc/dealloc

Regarding _skb_refdst memory alloc/dealloc

[Lokesh Dhoundiyal]

Hi,

I have the tunnel destination entry set via skb_dst_set inside 
ip_tunnel_rcv. I wish to release the memory referenced by 
skb->_skb_refdst after use.

Could you please advise the api to use for it. I am assuming that it is 
skb_dst_drop, Is that correct?

Cheers,
Lokesh

Re: Regarding _skb_refdst memory alloc/dealloc

[Lokesh Dhoundiyal]

Hi,

Sorry for my repeated emails.

To add some background to my earlier message and get some advise on things.

While testing the scenario of GRE over IPsec with routing table learned 
via dynamic protocol (OSPF) we have observed the below mentioned leak.

Kmemleak output:
unreferenced object 0x8000000044bebb00 (size 256):
   comm "softirq", pid 0, jiffies 4294985356 (age 126.810s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 80 00 00 00 05 13 74 80 ..............t.
     80 00 00 00 04 9b bf f9 00 00 00 00 00 00 00 00 ................
   backtrace:
     [<00000000f83947e0>] __kmalloc+0x1e8/0x300
     [<00000000b7ed8dca>] metadata_dst_alloc+0x24/0x58
     [<0000000081d32c20>] __ipgre_rcv+0x100/0x2b8
     [<00000000824f6cf1>] gre_rcv+0x178/0x540
     [<00000000ccd4e162>] gre_rcv+0x7c/0xd8
     [<00000000c024b148>] ip_protocol_deliver_rcu+0x124/0x350
     [<000000006a483377>] ip_local_deliver_finish+0x54/0x68
     [<00000000d9271b3a>] ip_local_deliver+0x128/0x168
     [<00000000bd4968ae>] xfrm_trans_reinject+0xb8/0xf8
     [<0000000071672a19>] tasklet_action_common.isra.16+0xc4/0x1b0
     [<0000000062e9c336>] __do_softirq+0x1fc/0x3e0
     [<00000000013d7914>] irq_exit+0xc4/0xe0
     [<00000000a4d73e90>] plat_irq_dispatch+0x7c/0x108
     [<000000000751eb8e>] handle_int+0x16c/0x178
     [<000000001668023b>] _raw_spin_unlock_irqrestore+0x1c/0x28

Investigation indicated that the change to the "if" statement in the 
commit c0d59da79534 results in the allocation and assignment happening 
for the tunnel destination which then later gets overwritten by 
skb_dst_set in ip_route_input_mc without freeing the original buffer. I 
am trying to free this skb->_skb_refdst buffer before the new buffer is set.

Could you please advise the api to use for it. I am assuming that it is 
skb_dst_drop, Is that correct?

Cheers,

Lokesh

On 3/05/22 3:10 pm, lokeshd wrote:
> Hi,
>
> I have the tunnel destination entry set via skb_dst_set inside 
> ip_tunnel_rcv. I wish to release the memory referenced by 
> skb->_skb_refdst after use.
>
> Could you please advise the api to use for it. I am assuming that it 
> is skb_dst_drop, Is that correct?
>
> Cheers,
> Lokesh

Re: Regarding _skb_refdst memory alloc/dealloc

[Chris Packham]

+ Dave and Wen

On 3/05/22 15:10, Lokesh Dhoundiyal wrote:

> Hi,
>
> I have the tunnel destination entry set via skb_dst_set inside
> ip_tunnel_rcv. I wish to release the memory referenced by
> skb->_skb_refdst after use.
>
> Could you please advise the api to use for it. I am assuming that it is
> skb_dst_drop, Is that correct?

A bit more context. We've been seeing a memory leak that seems to have 
appeared when we updated our Linux kernel from v4.4.16 to v5.7.19. The 
test scenario involves learning OSPF routes over a tunnel. I don't 
imagine there's anything particularly special about OSFP just that it 
uses multicast traffic to communicate.

Some debugging pointed us at the kmalloc-256 slab and kmemleak seemed to 
confirm the suspicion.

unreferenced object 0x8000000044beb900 (size 256):
   comm "softirq", pid 0, jiffies 4294984455 (age 35.980s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 80 00 00 00 05 13 74 80 ..............t.
     80 00 00 00 04 9b bf f9 00 00 00 00 00 00 00 00 ................
   backtrace:
     [<00000000f83947e0>] __kmalloc+0x1e8/0x300
     [<00000000b7ed8dca>] metadata_dst_alloc+0x24/0x58
     [<0000000081d32c20>] __ipgre_rcv+0x100/0x2b8
     [<00000000824f6cf1>] gre_rcv+0x178/0x540
     [<00000000ccd4e162>] gre_rcv+0x7c/0xd8
     [<00000000c024b148>] ip_protocol_deliver_rcu+0x124/0x350
     [<000000006a483377>] ip_local_deliver_finish+0x54/0x68
     [<00000000d9271b3a>] ip_local_deliver+0x128/0x168
     [<00000000bd4968ae>] xfrm_trans_reinject+0xb8/0xf8
     [<0000000071672a19>] tasklet_action_common.isra.16+0xc4/0x1b0
     [<0000000062e9c336>] __do_softirq+0x1fc/0x3e0
     [<00000000013d7914>] irq_exit+0xc4/0xe0
     [<00000000a4d73e90>] plat_irq_dispatch+0x7c/0x108
     [<000000000751eb8e>] handle_int+0x16c/0x178
     [<00000000a0c43b3e>] put_object+0x20/0xd8
     [<000000009439acbb>] scan_gray_list+0x18c/0x268

It appears that the leak is due to commit c0d59da79534 ("ip_gre: Make 
none-tun-dst gre tunnel store tunnel info as metadat_dst in recv"). 
Prior to c0d59da79534 we'd only allocate a new dst if tunnel->collect_md 
were true but now we'll also allocate one if tnl_params->daddr == 0. 
When ip_route_input_mc() is eventually called it will call skb_dst_set() 
leaking whatever is in skb->_skb_refdst.

A naive fix would be to call skb_dst_drop() in ip_route_input_mc() just 
before calling skb_dst_set() (hence Lokesh's question) but I'm worried 
we've missed something. I can't rule out that this has already been 
fixed or is due to other changes in our kernel fork. I can't see 
anything that says "Fixes: c0d59da79534" so if it has been fixed 
c0d59da79534 doesn't appear to have been noted as the culprit. I've 
asked Lokesh to try and reproduce the problem with the latest kernel so 
we can rule out any changes we've made and confirm that the leak still 
exists.

I wanted to get this out now just in case it rings any bells or if 
someone has got a tunnel+multicast setup that might show the problem.

Thanks,
Chris