Re: [PATCH V2] netrom: Prevent race conditions between multiple add route
From: Dan Carpenter <hidden>
Date: 2025-10-20 12:57:39
Also in:
linux-hams, lkml
On Mon, Oct 20, 2025 at 03:25:40PM +0300, Dan Carpenter wrote:
netrom/nr_route.c 214 nr_node_lock(nr_node);
I guess nr_node is different for each thread?
215
216 if (quality != 0)
217 strscpy(nr_node->mnemonic, mnemonic);
218
219 for (found = 0, i = 0; i < nr_node->count; i++) {
220 if (nr_node->routes[i].neighbour == nr_neigh) {
221 nr_node->routes[i].quality = quality;
222 nr_node->routes[i].obs_count = obs_count;
223 found = 1;
224 break;
225 }
226 }
227
228 if (!found) {
229 /* We have space at the bottom, slot it in */
230 if (nr_node->count < 3) {
231 nr_node->routes[2] = nr_node->routes[1];
232 nr_node->routes[1] = nr_node->routes[0];
233
234 nr_node->routes[0].quality = quality;
235 nr_node->routes[0].obs_count = obs_count;
236 nr_node->routes[0].neighbour = nr_neigh;
237
238 nr_node->which++;
239 nr_node->count++;
240 nr_neigh_hold(nr_neigh);
241 nr_neigh->count++;
242 } else {
243 /* It must be better than the worst */
244 if (quality > nr_node->routes[2].quality) {
245 nr_node->routes[2].neighbour->count--;
246 nr_neigh_put(nr_node->routes[2].neighbour);
247
248 if (nr_node->routes[2].neighbour->count == 0 && !nr_node->routes[2].neighbour->locked)
249 nr_remove_neigh(nr_node->routes[2].neighbour);Does neighbour->count means how many nr_node pointers have it in ->routes[]? I wish this code had comments. KTDOO: add comments explaining all the counters in netrom/nr_route.c
250 251 nr_node->routes[2].quality = quality; 252 nr_node->routes[2].obs_count = obs_count; 253 nr_node->routes[2].neighbour = nr_neigh; 254 255 nr_neigh_hold(nr_neigh); 256 nr_neigh->count++;
Could we just add some locking to this if statement only? I had thought that nr_node_lock() serialized it but now I think maybe not? Or maybe we could increment the counters before assigning it? nr_neigh->count++; nr_neigh_hold(nr_neigh); nr_node->routes[2].neighbour = nr_neigh; I'm not an expert in concerency. Does calling nr_neigh_hold() mean th that the nr_neigh->count++ will happen before the assignment?
257 } 258 } 259 }
regards, dan carpenter