libceph: check reply num_data_items in setup_request_data()
setup_request_data() adds message data items to both request and reply messages, but only checks request num_data_items before proceeding with the loop. This is wrong because if an op doesn't have any request data items but has a reply data item (e.g. read), a duplicate data item gets added to the message on every resend attempt. This went unnoticed for years but now that message data items are preallocated, it promptly crashes in ceph_msg_data_add(). Amend the signature to make it clear that setup_request_data() operates on both request and reply messages. Also, remove data_len assert -- we have another one in prepare_write_message(). Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This commit is contained in:
parent
0d9c1ab3be
commit
98c4bfe9d8
|
@ -1917,48 +1917,48 @@ static bool should_plug_request(struct ceph_osd_request *req)
|
|||
/*
|
||||
* Keep get_num_data_items() in sync with this function.
|
||||
*/
|
||||
static void setup_request_data(struct ceph_osd_request *req,
|
||||
struct ceph_msg *msg)
|
||||
static void setup_request_data(struct ceph_osd_request *req)
|
||||
{
|
||||
u32 data_len = 0;
|
||||
int i;
|
||||
struct ceph_msg *request_msg = req->r_request;
|
||||
struct ceph_msg *reply_msg = req->r_reply;
|
||||
struct ceph_osd_req_op *op;
|
||||
|
||||
if (msg->num_data_items)
|
||||
if (req->r_request->num_data_items || req->r_reply->num_data_items)
|
||||
return;
|
||||
|
||||
WARN_ON(msg->data_length);
|
||||
for (i = 0; i < req->r_num_ops; i++) {
|
||||
struct ceph_osd_req_op *op = &req->r_ops[i];
|
||||
|
||||
WARN_ON(request_msg->data_length || reply_msg->data_length);
|
||||
for (op = req->r_ops; op != &req->r_ops[req->r_num_ops]; op++) {
|
||||
switch (op->op) {
|
||||
/* request */
|
||||
case CEPH_OSD_OP_WRITE:
|
||||
case CEPH_OSD_OP_WRITEFULL:
|
||||
WARN_ON(op->indata_len != op->extent.length);
|
||||
ceph_osdc_msg_data_add(msg, &op->extent.osd_data);
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->extent.osd_data);
|
||||
break;
|
||||
case CEPH_OSD_OP_SETXATTR:
|
||||
case CEPH_OSD_OP_CMPXATTR:
|
||||
WARN_ON(op->indata_len != op->xattr.name_len +
|
||||
op->xattr.value_len);
|
||||
ceph_osdc_msg_data_add(msg, &op->xattr.osd_data);
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->xattr.osd_data);
|
||||
break;
|
||||
case CEPH_OSD_OP_NOTIFY_ACK:
|
||||
ceph_osdc_msg_data_add(msg,
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->notify_ack.request_data);
|
||||
break;
|
||||
|
||||
/* reply */
|
||||
case CEPH_OSD_OP_STAT:
|
||||
ceph_osdc_msg_data_add(req->r_reply,
|
||||
ceph_osdc_msg_data_add(reply_msg,
|
||||
&op->raw_data_in);
|
||||
break;
|
||||
case CEPH_OSD_OP_READ:
|
||||
ceph_osdc_msg_data_add(req->r_reply,
|
||||
ceph_osdc_msg_data_add(reply_msg,
|
||||
&op->extent.osd_data);
|
||||
break;
|
||||
case CEPH_OSD_OP_LIST_WATCHERS:
|
||||
ceph_osdc_msg_data_add(req->r_reply,
|
||||
ceph_osdc_msg_data_add(reply_msg,
|
||||
&op->list_watchers.response_data);
|
||||
break;
|
||||
|
||||
|
@ -1967,25 +1967,23 @@ static void setup_request_data(struct ceph_osd_request *req,
|
|||
WARN_ON(op->indata_len != op->cls.class_len +
|
||||
op->cls.method_len +
|
||||
op->cls.indata_len);
|
||||
ceph_osdc_msg_data_add(msg, &op->cls.request_info);
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->cls.request_info);
|
||||
/* optional, can be NONE */
|
||||
ceph_osdc_msg_data_add(msg, &op->cls.request_data);
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->cls.request_data);
|
||||
/* optional, can be NONE */
|
||||
ceph_osdc_msg_data_add(req->r_reply,
|
||||
ceph_osdc_msg_data_add(reply_msg,
|
||||
&op->cls.response_data);
|
||||
break;
|
||||
case CEPH_OSD_OP_NOTIFY:
|
||||
ceph_osdc_msg_data_add(msg,
|
||||
ceph_osdc_msg_data_add(request_msg,
|
||||
&op->notify.request_data);
|
||||
ceph_osdc_msg_data_add(req->r_reply,
|
||||
ceph_osdc_msg_data_add(reply_msg,
|
||||
&op->notify.response_data);
|
||||
break;
|
||||
}
|
||||
|
||||
data_len += op->indata_len;
|
||||
}
|
||||
|
||||
WARN_ON(data_len != msg->data_length);
|
||||
}
|
||||
|
||||
static void encode_pgid(void **p, const struct ceph_pg *pgid)
|
||||
|
@ -2033,7 +2031,7 @@ static void encode_request_partial(struct ceph_osd_request *req,
|
|||
req->r_data_offset || req->r_snapc);
|
||||
}
|
||||
|
||||
setup_request_data(req, msg);
|
||||
setup_request_data(req);
|
||||
|
||||
encode_spgid(&p, &req->r_t.spgid); /* actual spg */
|
||||
ceph_encode_32(&p, req->r_t.pgid.seed); /* raw hash */
|
||||
|
|
Loading…
Reference in New Issue