lkml.org 
[lkml]   [2003]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] 2.6.0 NBD driver: remove send/recieve race for request
Paul Clements wrote:

> . . .
>
>>Except that in the error case, the send basically didn't succeed. So no
>>need to worry about recieving a reply and no race possibility in that case.
>>
>>
>
>As long as the request is on the queue, it is possible for nbd-client to
>die, thus freeing the request (via nbd_clear_que -> nbd_end_request),
>and leaving us with a race between the free and do_nbd_request()
>accessing the request structure.
>
>--
>Paul
>
>
Quite right. I missed that case in this last patch (when nbd_do_it has
returned and NBD_DO_IT is about to call nbd_clear_que [1]). Just moving
the errors increment (near the end of nbd_send_req) to within the
semaphore protected region would fix this particular case. An even
larger race window exists with the request getting free'd when
nbd-client is used to disconnect in which it calls NBD_CLEAR_QUE before
NBD_DISCONNECT [2]. In this case, moving the errors increment doesn't
help of course since the nbd_clear_queue in 2.6.0-test2 doesn't bother
to check the tx_lock semaphore anyway. I believe reference counting the
request (as you suggest) would protect against both these windows though.

It's ironic that I'd fixed both these races [1+2] a ways back in an
earlier patch and had forgotten about these cases in this last patch I
submitted. The earlier patch p6.2 against linux-2.5.73 looks about
right. By that patch, the call to clear the queue before NBD_DO_IT
returned was gone and it made sure the clear_queue functionality would
return -EBUSY if invoked when the socket wasn't NULL (and potentially
while nbd_send_req functionality could be called). Not that I'm arguing
we should roll in these ealier patches again. That would re-introduce
the compatibility break which I wouldn't want either.

Will you be working on closing the other clear-queue race also then?
Here's the comments I shared on this in one of these earlier patches
that didn't make it into the mainstream distro (from patch #7):

/*

* Don't allow queue to be cleared while device is running!

* Device must be stopped (disconnected) first. Otherwise

* clearing is meaningless & can lock up processes: it's a

* race with users who may queue up more requests after the

* clearing is done that may then never be freed till the

* system reboots or clear que is run again which just

* opens the race yet again.

*/


-
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/

\
 
 \ /
  Last update: 2005-03-22 13:47    [W:2.985 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site