Messages in this thread | | | Subject | Re: A possible bug in reqsk_queue_hash_req() | From | Eric Dumazet <> | Date | Tue, 20 Apr 2010 13:06:51 +0200 |
| |
Le mardi 20 avril 2010 à 18:35 +0800, Li Yu a écrit : > Hi, > > I found out a possible bug in reqsk_queue_hash_req(), it seem > that we should move "req->dl_next = lopt->syn_table[hash];" statement > into follow write lock protected scope. > > As I browsed source code, this function only can be call at rx > code path which is protected a spin lock over struct sock , but its > caller ( inet_csk_reqsk_queue_hash_add() ) is a GPL exported symbol, > so I think that we'd best move this statement into below write lock > protected scope. > > Below is the patch to play this change, please do not apply it on > source code, it's just for show. > > Thanks. > > Yu > > --- include/net/request_sock.h 2010-04-09 15:27:14.000000000 +0800 > +++ include/net/request_sock.h 2010-04-20 18:11:32.000000000 +0800 > @@ -247,9 +247,9 @@ static inline void reqsk_queue_hash_req( > req->expires = jiffies + timeout; > req->retrans = 0; > req->sk = NULL; > - req->dl_next = lopt->syn_table[hash]; > > write_lock(&queue->syn_wait_lock); > + req->dl_next = lopt->syn_table[hash]; > lopt->syn_table[hash] = req; > write_unlock(&queue->syn_wait_lock); > }
I believe its not really necessary, because we are the only possible writer at this stage.
The write_lock() ... write_unlock() is there only to enforce a synchronisation with readers.
All callers of this reqsk_queue_hash_req() must have the socket locked
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |