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

[Graham, Simon]

And another one -- receive_DataRequest calls drbd_alloc_ee _before_
testing if it has a local disk; this craps out in bio_add_page because
bi_bdev is NULL (I think -- it's a bit hard to read the optimized
disassembly) -- certainly, when we go diskless because of an error,
after_state_ch frees the drbd_backing_dev and sets the bc pointer in
mdev to NULL -- a data request received after that will call
drbd_alloc_ee which will do the wrong thing.

I think the fix is to move the test for having good data to before the
call to drbd_alloc_ee; does that seem reasonable?

/simgr


> -----Original Message-----
> From: drbd-dev-bounces at linbit.com [mailto:drbd-dev-bounces at linbit.com]
> On Behalf Of Graham, Simon
> Sent: Saturday, August 05, 2006 10:29 PM
> To: drbd-dev at linbit.com
> Subject: [Drbd-dev] DRBD-8 trunk crashes if NegDReply received in
reply
> toRSDataRequest
> 
> 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
> 
> _______________________________________________
> drbd-dev mailing list
> drbd-dev at lists.linbit.com
> http://lists.linbit.com/mailman/listinfo/drbd-dev