[Drbd-dev] DRBD-8 trunk crashes if NegDReply received in reply to RSDataRequest

[Drbd-dev] DRBD-8 trunk crashes if NegDReply received in reply to RSDataRequest

[Graham, Simon]

When an RSDataRequest is sent, the block_id field in the request is set
to SYNCER_ID (-1) - if the receiver is unable to process the request
(e.g. if the receiver doesn't have good data) then it sends back a
NegDReply with the same block_id - on the origin side, got_NegDReply
attempts to validate the block_id value using drbd_pr_verify which
promptly crashes attempting to reference the master_bio field to get the
sector (drbd_req_get_sector()).

I found this testing my fixes for removing panic() calls on meta data
read/write failures but I thought it was worth bugging separately;
clearly this routine needs to validate the request pointer before
attempting to access it at all, but also we should have got a
NedRSDreply in this case - so there are at least two bugs here:

1. The target side should have send a NegRSDreply in this case;
receive_DataRequest should switch on h->command 
   to decide what response to send when it bails early
2. drbd_pr_verify should NOT call drbd_req_get_sector() before
validating the pointer - is there any reason why 
   it shouldn't use the sector value passed in as a parameter?

I can make patches for these when we agree on the right solution...

Simon

Re: [Drbd-dev] DRBD-8 trunk crashes if NegDReply received in reply to RSDataRequest

[Philipp Reisner]

Am Sonntag, 6. August 2006 04:29 schrieb Graham, Simon:
> When an RSDataRequest is sent, the block_id field in the request is set
> to SYNCER_ID (-1) - if the receiver is unable to process the request
> (e.g. if the receiver doesn't have good data) then it sends back a
> NegDReply with the same block_id - on the origin side, got_NegDReply
> attempts to validate the block_id value using drbd_pr_verify which
> promptly crashes attempting to reference the master_bio field to get the
> sector (drbd_req_get_sector()).
>
> I found this testing my fixes for removing panic() calls on meta data
> read/write failures but I thought it was worth bugging separately;
> clearly this routine needs to validate the request pointer before
> attempting to access it at all, but also we should have got a
> NedRSDreply in this case - so there are at least two bugs here:
>
> 1. The target side should have send a NegRSDreply in this case;
> receive_DataRequest should switch on h->command
>    to decide what response to send when it bails early

Right. 

> 2. drbd_pr_verify should NOT call drbd_req_get_sector() before
> validating the pointer - is there any reason why
>    it shouldn't use the sector value passed in as a parameter?

You are completely right. I guess this is a copy-n-paste error.
It should definitely use the passed parameter and not
call drbd_req_get_sector().

> I can make patches for these when we agree on the right solution...
>

I did the patching myself, in that cases. Please verify my
changes:
http://lists.linbit.com/pipermail/drbd-cvs/2006-August/001179.html

-Philipp
-- 
: Dipl-Ing Philipp Reisner                      Tel +43-1-8178292-50 :
: LINBIT Information Technologies GmbH          Fax +43-1-8178292-82 :
: Schönbrunnerstr 244, 1120 Vienna, Austria    http://www.linbit.com :