Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Protect against race conditions in VMDq config
This fixes race conditions in which two intel_mp apps could
end up using the same VLAN or MAC index (a TOCTOU bug). It
was difficult to trigger in a realistic situation, but was
possible to trigger when some delays were added in the code.
  • Loading branch information
takikawa committed Aug 16, 2017
1 parent c00adf8 commit ee0ce1d
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/apps/intel_mp/intel_mp.lua
Expand Up @@ -791,6 +791,10 @@ function Intel:add_receive_MAC (mac)

-- scan to see if the MAC is already recorded or find the
-- first free MAC index
--
-- the lock protects the critical section so that driver apps on
-- separate processes do not use conflicting registers
self:lock_sw_sem()
for idx=1, self.max_mac_addr do
local valid = self.r.RAH[idx]:bits(31, 1)

Expand All @@ -807,6 +811,7 @@ function Intel:add_receive_MAC (mac)
end
end
end
self:unlock_sw_sem()

assert(mac_index, "Max number of MAC addresses reached")

Expand All @@ -832,8 +837,8 @@ function Intel:add_receive_VLAN (vlan)
assert(vlan>=0 and vlan<4096, "bad VLAN number")
local vlan_index, first_empty

-- scan to see if the VLAN is already recorded or find the
-- first free VLAN index
-- works the same as add_receive_MAC
self:lock_sw_sem()
for idx=0, self.max_vlan-1 do
local valid = self.r.PFVLVF[idx]:bits(31, 1)

Expand All @@ -846,6 +851,7 @@ function Intel:add_receive_VLAN (vlan)
break
end
end
self:unlock_sw_sem()

if not vlan_index and first_empty then
vlan_index = first_empty
Expand Down
68 changes: 68 additions & 0 deletions src/apps/intel_mp/test_10g_vmdq_race.snabb
@@ -0,0 +1,68 @@
#!../../snabb snsh

-- Snabb test script that tests against race conditions in setting
-- VMDq parameters like MAC and VLAN

local C = require("ffi").C
local intel = require("apps.intel_mp.intel_mp")
local lib = require("core.lib")
local worker = require("core.worker")

local pciaddr0 = lib.getenv("SNABB_PCI_INTEL0")

-- launch two worker processes each using VMDq with MAC addresses
-- and different VLAN tags and ensure that the chosen indices for
-- MAC & VLAN do not overlap.
--
-- It's difficult for this test to fail unless delays are introduced
-- to deliberately trigger race conditions in the driver code (and
-- the locks disabled).
worker.start("worker0", [[
local intel = require("apps.intel_mp.intel_mp")
local lib = require("core.lib")
local pciaddr0 = lib.getenv("SNABB_PCI_INTEL0")
local c = config.new()
config.app(c, "nic0", intel.Intel,
{ pciaddr = pciaddr0,
vmdq = true,
poolnum = 0,
rxq = 0, txq = 0,
vlan = 0,
macaddr = "00:11:22:33:44:55" })
engine.configure(c)
engine.main({ duration = 1 })
assert(engine.app_table.nic0.r.RAH[1]:bits(31, 1) == 1)
assert(engine.app_table.nic0.r.PFVLVF[0]:bits(31, 1) == 1)
]])

worker.start("worker1", [[
local intel = require("apps.intel_mp.intel_mp")
local lib = require("core.lib")
local pciaddr0 = lib.getenv("SNABB_PCI_INTEL0")
local c = config.new()
config.app(c, "nic1", intel.Intel,
{ pciaddr = pciaddr0,
vmdq = true,
poolnum = 1,
rxq = 0, txq = 0,
vlan = 1,
macaddr = "55:44:33:22:11:00" })
engine.configure(c)
engine.main({ duration = 1 })
assert(engine.app_table.nic1.r.RAH[2]:bits(31, 1) == 1)
assert(engine.app_table.nic1.r.PFVLVF[1]:bits(31, 1) == 1)
]])

-- loop until all workers are done
while true do
local live = false
for w, s in pairs(worker.status()) do
live = live or s.alive
end

if live then
C.sleep(0.1)
else
break
end
end

0 comments on commit ee0ce1d

Please sign in to comment.