Project

General

Profile

Actions

Bug #235

closed

meta: Missing list checks for *list_add*

Added by Sven Eckelmann about 8 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
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 6 (0 open6 closed)

Bug #236: batadv_gw_node_update: Missing list checks for *list_add*Closedbatman-adv developers06/26/2015

Actions
Bug #237: batadv_neigh_node_new: Missing list checks for *list_add*ClosedMarek Lindner06/26/2015

Actions
Bug #238: batadv_tt_global_orig_entry_add: Missing list checks for *list_add*ClosedAntonio Quartulli06/26/2015

Actions
Bug #239: batadv_softif_create_vlan: Missing list checks for *list_add*ClosedAntonio Quartulli06/26/2015

Actions
Bug #240: batadv_nc_get_nc_node: Missing list checks for *list_add*ClosedMartin Hundebøll06/26/2015

Actions
Bug #241: batadv_tvlv_handler_register: Missing list checks for *list_add*Closedbatman-adv developers06/26/2015

Actions
Actions #1

Updated by Sven Eckelmann over 7 years ago

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

Updated by Sven Eckelmann about 7 years ago

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

Updated by Sven Eckelmann over 5 years ago

  • Status changed from New to In Progress
Actions #4

Updated by Sven Eckelmann over 5 years ago

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

Queued up for 2018.3

Actions #5

Updated by Sven Eckelmann over 5 years ago

  • Status changed from Resolved to Closed
Actions #6

Updated by Sven Eckelmann almost 4 years ago

  • Description updated (diff)
Actions

Also available in: Atom PDF