lkml.org 
[lkml]   [2003]   [Aug]   [6]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateWed, 06 Aug 2003 01:34:25 -0600
FromLou Langholtz <>
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    [from the cache]
©2003-2008