[DRBD-cvs] svn commit by lars - r2871 - trunk/drbd - fix for a
potential access-after-free in _drbd_send_zc_
drbd-cvs at lists.linbit.com
drbd-cvs at lists.linbit.com
Mon May 7 11:59:25 CEST 2007
Author: lars
Date: 2007-05-07 11:59:24 +0200 (Mon, 07 May 2007)
New Revision: 2871
Modified:
trunk/drbd/drbd_req.c
trunk/drbd/drbd_req.h
Log:
fix for a potential access-after-free in _drbd_send_zc_bio due to a race contition in _req_may_be_done
Modified: trunk/drbd/drbd_req.c
===================================================================
--- trunk/drbd/drbd_req.c 2007-05-06 20:02:07 UTC (rev 2870)
+++ trunk/drbd/drbd_req.c 2007-05-07 09:59:24 UTC (rev 2871)
@@ -33,6 +33,32 @@
#include "drbd_int.h"
#include "drbd_req.h"
+/* outside of the ifdef
+ * because of the _print_rq_state(,FIXME) in barrier_acked */
+void _print_rq_state(drbd_request_t *req, const char *txt)
+{
+ const unsigned long s = req->rq_state;
+ drbd_dev *mdev = req->mdev;
+ const int rw = (req->master_bio == NULL ||
+ bio_data_dir(req->master_bio) == WRITE) ?
+ 'W' : 'R';
+
+ INFO("%s %p %c L%c%c%cN%c%c%c%c%c %u (%llus +%u) %s\n",
+ txt, req, rw,
+ s & RQ_LOCAL_PENDING ? 'p' : '-',
+ s & RQ_LOCAL_COMPLETED ? 'c' : '-',
+ s & RQ_LOCAL_OK ? 'o' : '-',
+ s & RQ_NET_PENDING ? 'p' : '-',
+ s & RQ_NET_QUEUED ? 'q' : '-',
+ s & RQ_NET_SENT ? 's' : '-',
+ s & RQ_NET_DONE ? 'd' : '-',
+ s & RQ_NET_OK ? 'o' : '-',
+ req->epoch,
+ (unsigned long long)req->sector,
+ req->size,
+ conns_to_name(mdev->state.conn));
+}
+
//#define VERBOSE_REQUEST_CODE
#if defined(VERBOSE_REQUEST_CODE) || defined(ENABLE_DYNAMIC_TRACE)
void _print_req_mod(drbd_request_t *req,drbd_req_event_t what)
@@ -66,30 +92,6 @@
INFO("_req_mod(%p %c ,%s)\n", req, rw, rq_event_names[what]);
}
-void _print_rq_state(drbd_request_t *req, const char *txt)
-{
- const unsigned long s = req->rq_state;
- drbd_dev *mdev = req->mdev;
- const int rw = (req->master_bio == NULL ||
- bio_data_dir(req->master_bio) == WRITE) ?
- 'W' : 'R';
-
- INFO("%s %p %c L%c%c%cN%c%c%c%c%c %u (%llus +%u) %s\n",
- txt, req, rw,
- s & RQ_LOCAL_PENDING ? 'p' : '-',
- s & RQ_LOCAL_COMPLETED ? 'c' : '-',
- s & RQ_LOCAL_OK ? 'o' : '-',
- s & RQ_NET_PENDING ? 'p' : '-',
- s & RQ_NET_QUEUED ? 'q' : '-',
- s & RQ_NET_SENT ? 's' : '-',
- s & RQ_NET_DONE ? 'd' : '-',
- s & RQ_NET_OK ? 'o' : '-',
- req->epoch,
- (unsigned long long)req->sector,
- req->size,
- conns_to_name(mdev->state.conn));
-}
-
# ifdef ENABLE_DYNAMIC_TRACE
# define print_rq_state(R,T) MTRACE(TraceTypeRq,TraceLvlMetrics,_print_rq_state(R,T);)
# define print_req_mod(T,W) MTRACE(TraceTypeRq,TraceLvlMetrics,_print_req_mod(T,W);)
@@ -255,6 +257,16 @@
print_rq_state(req, "_req_may_be_done");
MUST_HOLD(&mdev->req_lock)
+ /* we must not complete the master bio, while it is
+ * still being processed by _drbd_send_zc_bio (drbd_send_dblock)
+ * not yet acknowledged by the peer
+ * not yet completed by the local io subsystem
+ * these flags may get cleared in any order by
+ * the worker,
+ * the receiver,
+ * the bio_endio completion callbacks.
+ */
+ if (s & RQ_NET_QUEUED) return;
if (s & RQ_NET_PENDING) return;
if (s & RQ_LOCAL_PENDING) return;
@@ -662,9 +674,7 @@
D_ASSERT(req->rq_state & RQ_NET_PENDING);
dec_ap_pending(mdev);
req->rq_state &= ~RQ_NET_PENDING;
- if (req->rq_state & RQ_NET_SENT)
- _req_may_be_done(req,error);
- /* else: done by handed_over_to_network */
+ _req_may_be_done(req,error);
break;
case neg_acked:
@@ -673,8 +683,7 @@
req->rq_state &= ~(RQ_NET_OK|RQ_NET_PENDING);
/* FIXME THINK! is it DONE now, or is it not? */
req->rq_state |= RQ_NET_DONE;
- if (req->rq_state & RQ_NET_SENT)
- _req_may_be_done(req,error);
+ _req_may_be_done(req,error);
/* else: done by handed_over_to_network */
break;
@@ -686,23 +695,7 @@
/* barrier came in before all requests have been acked.
* this is bad, because if the connection is lost now,
* we won't be able to clean them up... */
- const unsigned long s = req->rq_state;
- INFO("%s %p %c L%c%c%cN%c%c%c%c%c %u (%llus +%u) %s\n",
- "FIXME", req,
- /* in fact, it can only be a WRITE, but anyways */
- bio_data_dir(req->master_bio) == WRITE ? 'W' : 'R',
- s & RQ_LOCAL_PENDING ? 'p' : '-',
- s & RQ_LOCAL_COMPLETED ? 'c' : '-',
- s & RQ_LOCAL_OK ? 'o' : '-',
- s & RQ_NET_PENDING ? 'p' : '-',
- s & RQ_NET_QUEUED ? 'q' : '-',
- s & RQ_NET_SENT ? 's' : '-',
- s & RQ_NET_DONE ? 'd' : '-',
- s & RQ_NET_OK ? 'o' : '-',
- req->epoch,
- (unsigned long long)req->sector,
- req->size,
- conns_to_name(mdev->state.conn));
+ _print_rq_state(req, "FIXME (barrier_acked but pending)");
}
D_ASSERT(req->rq_state & RQ_NET_SENT);
req->rq_state |= RQ_NET_DONE;
@@ -714,11 +707,7 @@
dec_ap_pending(mdev);
req->rq_state &= ~RQ_NET_PENDING;
req->rq_state |= (RQ_NET_OK|RQ_NET_DONE);
- /* can it happen that we receive the DataReply
- * before the send DataRequest function returns? */
- if (req->rq_state & RQ_NET_SENT)
- _req_may_be_done(req,error);
- /* else: done by handed_over_to_network */
+ _req_may_be_done(req,error);
break;
};
}
Modified: trunk/drbd/drbd_req.h
===================================================================
--- trunk/drbd/drbd_req.h 2007-05-06 20:02:07 UTC (rev 2870)
+++ trunk/drbd/drbd_req.h 2007-05-07 09:59:24 UTC (rev 2871)
@@ -165,10 +165,16 @@
* no longer occur. */
__RQ_NET_QUEUED,
- /* well, actually only "handed over to the network stack" */
+ /* well, actually only "handed over to the network stack".
+ *
+ * TODO can potentially be dropped because of the similar meaning
+ * of RQ_NET_SENT and ~RQ_NET_QUEUED.
+ * however it is not exactly the same. before we drop it
+ * we must ensure that we can tell a request with network part
+ * from a request without, regardless of what happens to it. */
__RQ_NET_SENT,
- /* when set, the request may be freed.
+ /* when set, the request may be freed (if RQ_NET_QUEUED is clear).
* in (C) this happens when WriteAck is received,
* in (B,A) when the corresponding BarrierAck is received */
__RQ_NET_DONE,
@@ -180,6 +186,9 @@
/* peer called drbd_set_in_sync() for this write */
__RQ_NET_SIS,
+
+ /* keep this last, its for the RQ_NET_MASK */
+ __RQ_NET_MAX,
};
#define RQ_LOCAL_PENDING (1UL << __RQ_LOCAL_PENDING)
@@ -195,7 +204,7 @@
#define RQ_NET_OK (1UL << __RQ_NET_OK)
#define RQ_NET_SIS (1UL << __RQ_NET_SIS)
-#define RQ_NET_MASK (((RQ_NET_OK << 1)-1) & ~RQ_LOCAL_MASK) /* 0xf8 */
+#define RQ_NET_MASK (((1UL << __RQ_NET_MAX)-1) & ~RQ_LOCAL_MASK) /* 0x1f8 */
/* epoch entries */
static inline struct hlist_head* ee_hash_slot(drbd_dev *mdev, sector_t sector)
More information about the drbd-cvs
mailing list