Project

General

Profile

Bug #2064

vnic sets up protection before upper mac leading to panic

Added by Robert Mustacchi almost 9 years ago. Updated about 8 years ago.

Status:
Resolved
Priority:
High
Category:
kernel
Start date:
2012-02-02
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

A user with SmartOS reported the following panic:

ffffff002e4e4890 i_mac_notify+0x2f(0, a)
ffffff002e4e48e0 mac_protect_set+0xa7(ffffff0706aa32a0, ffffff072a1c7044)
ffffff002e4e4930 mac_client_set_resources+0xb3(ffffff0706aa32a0, ffffff072a1c7044)
ffffff002e4e4b00 vnic_dev_create+0x4c6(13, 1, ffffff002e4e4b84, ffffff002e4e4b8c, ffffff002e4e4b60, ffffff002e4e4b88, ffffff060000000c, ffffff0600000002, ffffff0000000000, ffffffff00000000, ffffff072a1c7044, ffffff0600000000, 
ffffff002e4e4b80, ffffff0703059698)
ffffff002e4e4be0 vnic_ioc_create+0xfd(ffffff072a1c7000, 80415d0, 100003, ffffff0703059698, ffffff002e4e4e68)
ffffff002e4e4c80 drv_ioctl+0x1e4()
ffffff002e4e4cc0 cdev_ioctl+0x39()
ffffff002e4e4d10 spec_ioctl+0x60()
ffffff002e4e4da0 fop_ioctl+0x55(ffffff06f817f480, 1710001, 80415d0, 100003, ffffff0703059698, ffffff002e4e4e68, 0)
ffffff002e4e4ec0 ioctl+0x9b(3, 1710001, 80415d0)
ffffff002e4e4f10 _sys_sysenter_post_swapgs+0x149()

The following is the analysis from Max Bruning:

This is easily reproducible. I ran:

# dladm create-vnic -l igb1 -m b2:ab:e5:1c:a4:a2 -v 2 -p protection=restricted test1

And the system immediately paniced. From looking at the code, the "-p protection=restricted"
causes mac_protect_set to be called (as in the stack trace above). However, the first argument passed
to i_mac_notify() is NULL. From

2210:    i_mac_notify(((mcip->mci_state_flags & MCIS_IS_VNIC) != 0 ?
2211:        mcip->mci_upper_mip : mip), MAC_NOTE_ALLOWED_IPS); 

In this case, the MCIS_IS_VNIC flag is set, so the argument passed is mcip->mci_upper_mip.
However:

> ffffff0706aa32a0::print -t mac_client_impl_t
mac_client_impl_t {
    struct mac_client_impl_s *mci_client_next = 0xffffff06fd95d008
    char [256] mci_name = [ "tboot0" ]
    flow_entry_t *mci_flent = 0xffffff072c0cf3e0
    struct mac_impl_s *mci_mip = 0xffffff06fe012a08
    struct mac_impl_s *mci_upper_mip = 0
...

So the value is NULL. It is initialized in the vnic_dev_create() function by calling
mac_set_upper_mac(), but that is done after the call to mac_client_set_resources.

In uts/common/io/vnic/vnic_dev.c:

328:  int
329:  vnic_dev_create(datalink_id_t vnic_id, datalink_id_t linkid,
330:      vnic_mac_addr_type_t *vnic_addr_type, int *mac_len, uchar_t *mac_addr,
331:      int *mac_slot, uint_t mac_prefix_len, uint16_t vid, vrid_t vrid,
332:      int af, mac_resource_props_t *mrp, uint32_t flags, vnic_ioc_diag_t *diag,
333:      cred_t *credp)
334:  {
...
408:        if (mrp != NULL) {
409:            if ((mrp->mrp_mask & MRP_RX_RINGS) != 0 ||
410:                (mrp->mrp_mask & MRP_TX_RINGS) != 0) {
411:                req_hwgrp_flag = B_TRUE;
412:            }
413:            err = mac_client_set_resources(vnic->vn_mch, mrp);
414:            if (err != 0)
415:                goto bail;
416:        }
...
519:    /* Set the VNIC's MAC in the client */
520:    if (!is_anchor)
521:        mac_set_upper_mac(vnic->vn_mch, vnic->vn_mh, mrp);
...

As mentioned earlier, mac_set_upper_mac() initializes mci_upper_mip (the value that is NULL at the time of the panic).

To fix this, we can't simply move the call of mac_set_upper_mac() to before the call to mac_client_set_resources() because
vnic->vn_mh is not initialized until calling mac_register(), which occurs at line 511.

One fix might be to move lines 408 through 416 to after line 521. I need to look at that closely, as I suspect some
cleanup would need to be added on failure (i.e., before the goto bail).
Another fix would be to disallow setting protections until the vnic has been created. But setting protections on creation
seems like a reasonable thing to do.

#1

Updated by Robert Mustacchi about 8 years ago

  • Subject changed from Panic in i_mac_notify to vnic sets up protection before upper mac leading to panic
#2

Updated by Robert Mustacchi about 8 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100
  • Tags deleted (needs-triage)

Resolved in 13843:ecae50d1b4e7. This work was done by Max Bruning.

Also available in: Atom PDF