Project

General

Profile

Bug #235

meta: Missing list checks for *list_add*

Added by Sven Eckelmann about 5 years ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
batman-adv developers
Target version:
Start date:
06/26/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)

Description

Simon debugged the refcnt problem and submitted some patches to fix them. I had a brief look and noticed that there are possible more problems similar to the *list_del* ones - just with *list_add*. Basically some functions use some kind of get function, notice that the element does not exist and then create a new one to add to the list. Only the "list_add" is protected. The result may be that an element in twice in a list when only a single occurrence is allowed.

The problem I saw is that functions adding objects in an RCU protected list are missing an definitive check. They first call some kind of *_get (rcu_read_lock only) to check if an object with this value already exists and then uses some kind of *_add to allocate a new object and add it (which may already be added in by a different context). So it has to be made sure that nothing modifies the list between the check and the add of the new object).

There are two common strategies (they are more):

  1. Fast *_get-check and on failure do locking
    • do a fast check with only rcu_read_lock
      • return object when found and reference could be increased
      • otherwise continue in function
    • get spinlock for list
    • do the same _get check (but this time with the spinlock)
      • return object when found and reference could be increased (unlock spinlock)
      • otherwise continue in function
    • allocate/initialize new object
    • add it to the list
    • return newly allocated object (unlock spinlock)
  2. Fast *_get-check and on failure do allocating and check afterwards with locking
    • do a fast check with only rcu_read_lock
      • return object when found and reference could be increased
      • otherwise continue in function
    • allocate/initialize new object
    • get spinlock for list
    • do the same _get check (but this time with the spinlock)
      • return object when found and reference could be increased (unlock spinlock); deallocate newly allocated object
      • otherwise continue in function
    • add it to the list
    • return newly allocated object (unlock spinlock)

batman-adv already uses both strategies in the code

Just for illustration what I meant with nothing modifies the list between the check and the add of the new object


Subtasks

Bug #236: batadv_gw_node_update: Missing list checks for *list_add*Closedbatman-adv developers

Actions
Bug #237: batadv_neigh_node_new: Missing list checks for *list_add*ClosedMarek Lindner

Actions
Bug #238: batadv_tt_global_orig_entry_add: Missing list checks for *list_add*ClosedAntonio Quartulli

Actions
Bug #239: batadv_softif_create_vlan: Missing list checks for *list_add*ClosedAntonio Quartulli

Actions
Bug #240: batadv_nc_get_nc_node: Missing list checks for *list_add*ClosedMartin Hundebøll

Actions
Bug #241: batadv_tvlv_handler_register: Missing list checks for *list_add*Closedbatman-adv developers

Actions

History

#1

Updated by Sven Eckelmann over 4 years ago

  • Subject changed from Missing list checks for *list_add* to meta: Missing list checks for *list_add*
#2

Updated by Sven Eckelmann about 4 years ago

  • Assignee changed from Marek Lindner to batman-adv developers
#3

Updated by Sven Eckelmann over 2 years ago

  • Status changed from New to In Progress
#4

Updated by Sven Eckelmann over 2 years ago

  • Target version set to 2018.3
  • Status changed from In Progress to Resolved

Queued up for 2018.3

#5

Updated by Sven Eckelmann over 2 years ago

  • Status changed from Resolved to Closed
#6

Updated by Sven Eckelmann 11 months ago

  • Description updated (diff)

Also available in: Atom PDF