Skip to content

Commit 98c4bfe

Browse files
idryomovidryomov
committed
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>
1 parent 0d9c1ab commit 98c4bfe

File tree

1 file changed

+23
-25
lines changed

1 file changed

+23
-25
lines changed

‎net/ceph/osd_client.c

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,48 +1917,48 @@ static bool should_plug_request(struct ceph_osd_request *req)
19171917
/*
19181918
* Keep get_num_data_items() in sync with this function.
19191919
*/
1920-
static void setup_request_data(struct ceph_osd_request *req,
1921-
struct ceph_msg *msg)
1920+
static void setup_request_data(struct ceph_osd_request *req)
19221921
{
1923-
u32 data_len = 0;
1924-
int i;
1922+
struct ceph_msg *request_msg = req->r_request;
1923+
struct ceph_msg *reply_msg = req->r_reply;
1924+
struct ceph_osd_req_op *op;
19251925

1926-
if (msg->num_data_items)
1926+
if (req->r_request->num_data_items || req->r_reply->num_data_items)
19271927
return;
19281928

1929-
WARN_ON(msg->data_length);
1930-
for (i = 0; i < req->r_num_ops; i++) {
1931-
struct ceph_osd_req_op *op = &req->r_ops[i];
1932-
1929+
WARN_ON(request_msg->data_length || reply_msg->data_length);
1930+
for (op = req->r_ops; op != &req->r_ops[req->r_num_ops]; op++) {
19331931
switch (op->op) {
19341932
/* request */
19351933
case CEPH_OSD_OP_WRITE:
19361934
case CEPH_OSD_OP_WRITEFULL:
19371935
WARN_ON(op->indata_len != op->extent.length);
1938-
ceph_osdc_msg_data_add(msg, &op->extent.osd_data);
1936+
ceph_osdc_msg_data_add(request_msg,
1937+
&op->extent.osd_data);
19391938
break;
19401939
case CEPH_OSD_OP_SETXATTR:
19411940
case CEPH_OSD_OP_CMPXATTR:
19421941
WARN_ON(op->indata_len != op->xattr.name_len +
19431942
op->xattr.value_len);
1944-
ceph_osdc_msg_data_add(msg, &op->xattr.osd_data);
1943+
ceph_osdc_msg_data_add(request_msg,
1944+
&op->xattr.osd_data);
19451945
break;
19461946
case CEPH_OSD_OP_NOTIFY_ACK:
1947-
ceph_osdc_msg_data_add(msg,
1947+
ceph_osdc_msg_data_add(request_msg,
19481948
&op->notify_ack.request_data);
19491949
break;
19501950

19511951
/* reply */
19521952
case CEPH_OSD_OP_STAT:
1953-
ceph_osdc_msg_data_add(req->r_reply,
1953+
ceph_osdc_msg_data_add(reply_msg,
19541954
&op->raw_data_in);
19551955
break;
19561956
case CEPH_OSD_OP_READ:
1957-
ceph_osdc_msg_data_add(req->r_reply,
1957+
ceph_osdc_msg_data_add(reply_msg,
19581958
&op->extent.osd_data);
19591959
break;
19601960
case CEPH_OSD_OP_LIST_WATCHERS:
1961-
ceph_osdc_msg_data_add(req->r_reply,
1961+
ceph_osdc_msg_data_add(reply_msg,
19621962
&op->list_watchers.response_data);
19631963
break;
19641964

@@ -1967,25 +1967,23 @@ static void setup_request_data(struct ceph_osd_request *req,
19671967
WARN_ON(op->indata_len != op->cls.class_len +
19681968
op->cls.method_len +
19691969
op->cls.indata_len);
1970-
ceph_osdc_msg_data_add(msg, &op->cls.request_info);
1970+
ceph_osdc_msg_data_add(request_msg,
1971+
&op->cls.request_info);
19711972
/* optional, can be NONE */
1972-
ceph_osdc_msg_data_add(msg, &op->cls.request_data);
1973+
ceph_osdc_msg_data_add(request_msg,
1974+
&op->cls.request_data);
19731975
/* optional, can be NONE */
1974-
ceph_osdc_msg_data_add(req->r_reply,
1976+
ceph_osdc_msg_data_add(reply_msg,
19751977
&op->cls.response_data);
19761978
break;
19771979
case CEPH_OSD_OP_NOTIFY:
1978-
ceph_osdc_msg_data_add(msg,
1980+
ceph_osdc_msg_data_add(request_msg,
19791981
&op->notify.request_data);
1980-
ceph_osdc_msg_data_add(req->r_reply,
1982+
ceph_osdc_msg_data_add(reply_msg,
19811983
&op->notify.response_data);
19821984
break;
19831985
}
1984-
1985-
data_len += op->indata_len;
19861986
}
1987-
1988-
WARN_ON(data_len != msg->data_length);
19891987
}
19901988

19911989
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,
20332031
req->r_data_offset || req->r_snapc);
20342032
}
20352033

2036-
setup_request_data(req, msg);
2034+
setup_request_data(req);
20372035

20382036
encode_spgid(&p, &req->r_t.spgid); /* actual spg */
20392037
ceph_encode_32(&p, req->r_t.pgid.seed); /* raw hash */

0 commit comments

Comments
 (0)