Project

General

Profile

Actions

Bug #3914

closed

ill_frag_hash_tbl not allocated for loopback interfaces

Added by Joshua M. Clulow almost 8 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Category:
networking
Start date:
2013-07-27
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

> ::status
debugging crash dump vmcore.0 (64-bit)
operating system: 5.11 joyent (i86pc)
panic message:
BAD TRAP: type=e (#pf Page fault) rp=ffffff00b8d93070 addr=710 occurred in module "unix" due to a NULL pointer dereference
dump content: kernel pages only

> ::stack
mutex_enter+0xb()
ire_recv_local_v4+0x16d(ffffff19e50e2b00, ffffff1de13e4f00, ffffff19bd255014, ffffff00b8d933b0)
ill_input_short_v4+0x6ce(ffffff1de13e4f00, ffffff19bd255014, ffffff19bd255024, ffffff00b8d933b0, ffffff00b8d93540)
ip_input+0x23b(ffffff19bfe29528, 0, ffffff1de13e4f00, ffffff00b8d93610)
i_dls_link_rx+0x2e7(ffffff19990f8008, 0, ffffff1de13e4f00, 0)
mac_rx_deliver+0x5d(ffffff19becfb560, 0, ffffff1de13e4f00, 0)
mac_rx_soft_ring_process+0x17a(ffffff19becfb560, ffffff19bec61900, ffffff1de13e4f00, ffffff1de13e4f00, 1, 0)
mac_rx_srs_proto_fanout+0x4e5(ffffff19becda9c0, ffffff1de13e4f00)
mac_rx_srs_drain+0x26e(ffffff19becda9c0, 800)
mac_rx_srs_process+0x1a9(ffffff199f7be010, ffffff19becda9c0, ffffff1de13e4f00, 0)
mac_rx_classify+0x159(ffffff199f7be010, ffffff19b310ccc8, ffffff1de13e4f00)
mac_rx_flow+0x54(ffffff199f7be010, ffffff19b310ccc8, ffffff1de13e4f00)
mac_rx_common+0x1f6(ffffff199f7be010, ffffff19b310ccc8, ffffff1de13e4f00)
mac_rx+0xac(ffffff199f7be010, ffffff19b310ccc8, ffffff1de13e4f00)
mac_rx_ring+0x4c(ffffff199f7be010, ffffff19b310ccc8, ffffff1de13e4f00, 0)
igb_intr_rx+0x5e(ffffff19b310e510, 0)
av_dispatch_autovect+0x7c(43)
dispatch_hardint+0x33(43, 0)
switch_sp_and_call+0x13()
do_interrupt+0xb8(ffffff00b8d5dac0, 0)
_interrupt+0xba()
i86_mwait+0xd()
cpu_idle_mwait+0xf1()
idle+0x114()
thread_start+8()

Note that, because we died in mutex_enter(), we're missing a stack frame -- reconstructed here:

ffffff00b8d93220 ip_input_fragment+0x38b(ffffff1de13e4f00, ffffff19bd255014
ffffff00b8d933b0)
Actions #1

Updated by Joshua M. Clulow almost 8 years ago

Digging in to the source... the call itself:

 6990 mblk_t *
 6991 ip_input_fragment(mblk_t *mp, ipha_t *ipha, ip_recv_attr_t *ira)
...
 7014         ill_t           *ill = ira->ira_ill;
...
 7129         ipfb = &ill->ill_frag_hash_tbl[ILL_FRAG_HASH(src, ident)];
 7130         mutex_enter(&ipfb->ipfb_lock);

Find ill, which we believe to be ffffff00b8d933b0, which would be up in the
stack frame for the call to ip_input/ip_input_common_v4 -- this appears to make sense.

> ffffff00b8d933b0::print ip_recv_attr_t ira_ill
{
    ill_inputfn = ill_input_short_v4
...
    ill_name = 0xffffff19b4282be0 "lo0" 
...
    ill_frag_hash_tbl = 0

Looking back at the source:

 105 #define ILL_FRAG_HASH_TBL_COUNT ((unsigned int)64)
 106 #define ILL_FRAG_HASH_TBL_SIZE  (ILL_FRAG_HASH_TBL_COUNT * sizeof (ipfb_t))
...
2748 #define ILL_FRAG_HASH(s, i) \\
2749         ((ntohl(s) ^ ((i) ^ ((i) >> 8))) % ILL_FRAG_HASH_TBL_COUNT)

So, we need src and ident:

> 0xffffff19bd255014::print -ta ipha_t
  4 ffffff19bd255018 uint16_t ipha_ident = 0xe60a
  c ffffff19bd255020 ipaddr_t ipha_src = 2.231.19.148
...
src = 0x9413e702 --ntohl--> 0x02e71394
ident = 0xe60a

...
> ::sizeof ipfb_t
sizeof (ipfb_t) = 0x20

... (python)
>>> hex((0x02e71394 ^ ((0xe60a) ^ ((0xe60a) >> 8))) % 64)
'0x38'

> 0x38 * 0x20 = X
                700

And there's our crazy offset from the panic.

Digging further into the source, it seems like we generally only set (or unset) ill_frag_hash_tbl in one allocation (and free) path:

Assignments to: ill_frag_hash_tbl

  File    Function        Line
1 ip_if.c ill_delete_tail  609 ill->ill_frag_hash_tbl = NULL;
2 ip_if.c ill_init        3379 ill->ill_frag_hash_tbl = (ipfb_t *)frag_ptr;

The usual call graph for allocation looks something like:

ip_openv4()
  --> ip_open(... MODOPEN ...)
      --> ip_modopen()
          --> mi_open_alloc_sleep(sizeof (ill_t))
          --> ill_init()
              --> assign ill_frag_hash_tbl

At which point ill_frag_hash_tbl is populated and all is well.

There is special case code, though, for the lo0 loopback adapter. These appear to be created with some oddball properties in response to a lookup for same, in ill_lookup_on_name if called with do_alloc B_TRUE.

As it turns out, we only call with do_alloc B_TRUE via:

ip_process_ioctl(SIOCLIFADDIF)
  --> ip_sioctl_addif() /* for interface matching loopback name, i.e. lo* */
      --> ipif_lookup_on_name()

The panic is the combination of:

  • ifconfig lo0 addif X.X.X.X up, in order to make a public IP address known as one of a zone's identities, but without putting that IP on a real interface
  • a packet large enough to be fragmented is routed to the zone, say via a NPAT or DSR load balancing setup

This is generally a load balanced public IP setup where the actual public IP address is mastered on a network device which then routes (without changing source or destination IP addresses) the packets to the zone. So that the IP stack in the zone does not immediately discard this otherwise foreign destination, it is brought up on the loopback adapter, where applications in the zone can bind to it.

This occurs because ire_recv_local_v4 notices that the ingress ILL (e.g. net0) for the received packet is not the same as the ILL that hosts the target IP address (lo0). In this event, we use ip_check_multihome to swap in the target ILL (assuming such a thing is allowed) before calling into ip_input_local_v4.

For fragmented packets, we then call into ip_input_fragment for processing. This function expects that "each ILL has a hash table for queuing packets undergoing reassembly for all IPIFs associated with the ILL." As noted in earlier comments, we most definitely do not allocate this hash table for lo0 as allocated inline with a ill_lookup_on_name() call.

The fix, here, is to allocate a hash table for the new lo0 ill_t in ill_lookup_on_name() by rejigging that function to use the appropriate parts of ill_init().

Actions #2

Updated by Electric Monk about 6 years ago

  • Status changed from New to Closed
  • % Done changed from 90 to 100

git commit f949c3862e64cd157f2a1aeefd0da08100f76173

commit  f949c3862e64cd157f2a1aeefd0da08100f76173
Author: Joshua M. Clulow <jmc@joyent.com>
Date:   2015-05-01T19:04:02.000Z

    3914 ill_frag_hash_tbl not allocated for loopback interfaces
    Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
    Approved by: Dan McDonald <danmcd@nexenta.com>

Actions

Also available in: Atom PDF