Bug #235
Updated by Sven Eckelmann almost 4 years ago
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 <code>*list_del*</code> ones - just with <code>*list_add*</code>. 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 "<code>list_add</code>" 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 <code>*_get</code> (<code>rcu_read_lock</code> only) to check if an object with this value already exists and then uses some kind of <code>*_add</code> 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): # 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) # 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":https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/1898798.WP4bC1arXA@bentobox/ object": https://patchwork.open-mesh.org/patch/4538/