diff options
author | Ilya Dryomov <idryomov@gmail.com> | 2018-10-17 08:23:04 -0400 |
---|---|---|
committer | Ilya Dryomov <idryomov@gmail.com> | 2018-10-22 04:28:23 -0400 |
commit | 98c4bfe9d89b22d7bfddf6469241658920b6fafe (patch) | |
tree | ae744f24466851329323f80dc69164d1602c82bf /net/ceph | |
parent | 0d9c1ab3be4c0187663096a6a084421d0a1e45c6 (diff) |
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>
Diffstat (limited to 'net/ceph')
-rw-r--r-- | net/ceph/osd_client.c | 48 |
1 files changed, 23 insertions, 25 deletions
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index cf0bd2cce848..a0148de51cc6 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c | |||
@@ -1917,48 +1917,48 @@ static bool should_plug_request(struct ceph_osd_request *req) | |||
1917 | /* | 1917 | /* |
1918 | * Keep get_num_data_items() in sync with this function. | 1918 | * Keep get_num_data_items() in sync with this function. |
1919 | */ | 1919 | */ |
1920 | static void setup_request_data(struct ceph_osd_request *req, | 1920 | static void setup_request_data(struct ceph_osd_request *req) |
1921 | struct ceph_msg *msg) | ||
1922 | { | 1921 | { |
1923 | u32 data_len = 0; | 1922 | struct ceph_msg *request_msg = req->r_request; |
1924 | int i; | 1923 | struct ceph_msg *reply_msg = req->r_reply; |
1924 | struct ceph_osd_req_op *op; | ||
1925 | 1925 | ||
1926 | if (msg->num_data_items) | 1926 | if (req->r_request->num_data_items || req->r_reply->num_data_items) |
1927 | return; | 1927 | return; |
1928 | 1928 | ||
1929 | WARN_ON(msg->data_length); | 1929 | WARN_ON(request_msg->data_length || reply_msg->data_length); |
1930 | for (i = 0; i < req->r_num_ops; i++) { | 1930 | for (op = req->r_ops; op != &req->r_ops[req->r_num_ops]; op++) { |
1931 | struct ceph_osd_req_op *op = &req->r_ops[i]; | ||
1932 | |||
1933 | switch (op->op) { | 1931 | switch (op->op) { |
1934 | /* request */ | 1932 | /* request */ |
1935 | case CEPH_OSD_OP_WRITE: | 1933 | case CEPH_OSD_OP_WRITE: |
1936 | case CEPH_OSD_OP_WRITEFULL: | 1934 | case CEPH_OSD_OP_WRITEFULL: |
1937 | WARN_ON(op->indata_len != op->extent.length); | 1935 | 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); | ||
1939 | break; | 1938 | break; |
1940 | case CEPH_OSD_OP_SETXATTR: | 1939 | case CEPH_OSD_OP_SETXATTR: |
1941 | case CEPH_OSD_OP_CMPXATTR: | 1940 | case CEPH_OSD_OP_CMPXATTR: |
1942 | WARN_ON(op->indata_len != op->xattr.name_len + | 1941 | WARN_ON(op->indata_len != op->xattr.name_len + |
1943 | op->xattr.value_len); | 1942 | 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); | ||
1945 | break; | 1945 | break; |
1946 | case CEPH_OSD_OP_NOTIFY_ACK: | 1946 | case CEPH_OSD_OP_NOTIFY_ACK: |
1947 | ceph_osdc_msg_data_add(msg, | 1947 | ceph_osdc_msg_data_add(request_msg, |
1948 | &op->notify_ack.request_data); | 1948 | &op->notify_ack.request_data); |
1949 | break; | 1949 | break; |
1950 | 1950 | ||
1951 | /* reply */ | 1951 | /* reply */ |
1952 | case CEPH_OSD_OP_STAT: | 1952 | case CEPH_OSD_OP_STAT: |
1953 | ceph_osdc_msg_data_add(req->r_reply, | 1953 | ceph_osdc_msg_data_add(reply_msg, |
1954 | &op->raw_data_in); | 1954 | &op->raw_data_in); |
1955 | break; | 1955 | break; |
1956 | case CEPH_OSD_OP_READ: | 1956 | case CEPH_OSD_OP_READ: |
1957 | ceph_osdc_msg_data_add(req->r_reply, | 1957 | ceph_osdc_msg_data_add(reply_msg, |
1958 | &op->extent.osd_data); | 1958 | &op->extent.osd_data); |
1959 | break; | 1959 | break; |
1960 | case CEPH_OSD_OP_LIST_WATCHERS: | 1960 | case CEPH_OSD_OP_LIST_WATCHERS: |
1961 | ceph_osdc_msg_data_add(req->r_reply, | 1961 | ceph_osdc_msg_data_add(reply_msg, |
1962 | &op->list_watchers.response_data); | 1962 | &op->list_watchers.response_data); |
1963 | break; | 1963 | break; |
1964 | 1964 | ||
@@ -1967,25 +1967,23 @@ static void setup_request_data(struct ceph_osd_request *req, | |||
1967 | WARN_ON(op->indata_len != op->cls.class_len + | 1967 | WARN_ON(op->indata_len != op->cls.class_len + |
1968 | op->cls.method_len + | 1968 | op->cls.method_len + |
1969 | op->cls.indata_len); | 1969 | 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); | ||
1971 | /* optional, can be NONE */ | 1972 | /* 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); | ||
1973 | /* optional, can be NONE */ | 1975 | /* optional, can be NONE */ |
1974 | ceph_osdc_msg_data_add(req->r_reply, | 1976 | ceph_osdc_msg_data_add(reply_msg, |
1975 | &op->cls.response_data); | 1977 | &op->cls.response_data); |
1976 | break; | 1978 | break; |
1977 | case CEPH_OSD_OP_NOTIFY: | 1979 | case CEPH_OSD_OP_NOTIFY: |
1978 | ceph_osdc_msg_data_add(msg, | 1980 | ceph_osdc_msg_data_add(request_msg, |
1979 | &op->notify.request_data); | 1981 | &op->notify.request_data); |
1980 | ceph_osdc_msg_data_add(req->r_reply, | 1982 | ceph_osdc_msg_data_add(reply_msg, |
1981 | &op->notify.response_data); | 1983 | &op->notify.response_data); |
1982 | break; | 1984 | break; |
1983 | } | 1985 | } |
1984 | |||
1985 | data_len += op->indata_len; | ||
1986 | } | 1986 | } |
1987 | |||
1988 | WARN_ON(data_len != msg->data_length); | ||
1989 | } | 1987 | } |
1990 | 1988 | ||
1991 | static void encode_pgid(void **p, const struct ceph_pg *pgid) | 1989 | 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, | |||
2033 | req->r_data_offset || req->r_snapc); | 2031 | req->r_data_offset || req->r_snapc); |
2034 | } | 2032 | } |
2035 | 2033 | ||
2036 | setup_request_data(req, msg); | 2034 | setup_request_data(req); |
2037 | 2035 | ||
2038 | encode_spgid(&p, &req->r_t.spgid); /* actual spg */ | 2036 | encode_spgid(&p, &req->r_t.spgid); /* actual spg */ |
2039 | ceph_encode_32(&p, req->r_t.pgid.seed); /* raw hash */ | 2037 | ceph_encode_32(&p, req->r_t.pgid.seed); /* raw hash */ |