libceph: fix potential use-after-free on linger ping and resends

request_reinit() is not only ugly as the comment rightfully suggests, but also unsafe. Even though it is called with osdc->lock held for write in all cases, resetting the OSD request refcount can still race with handle_reply() and result in use-after-free. Taking linger ping as an example: handle_timeout thread handle_reply thread down_read(&osdc->lock) req = lookup_request(...) ... finish_request(req) # unregisters up_read(&osdc->lock) __complete_request(req) linger_ping_cb(req) # req->r_kref == 2 because handle_reply still holds its ref down_write(&osdc->lock) send_linger_ping(lreq) req = lreq->ping_req # same req # cancel_linger_request is NOT # called - handle_reply already # unregistered request_reinit(req) WARN_ON(req->r_kref != 1) # fires request_init(req) kref_init(req->r_kref) # req->r_kref == 1 after kref_init ceph_osdc_put_request(req) kref_put(req->r_kref) # req->r_kref == 0 after kref_put, req is freed <further req initialization/use> !!! This happens because send_linger_ping() always (re)uses the same OSD request for watch ping requests, relying on cancel_linger_request() to unregister it from the OSD client and rip its messages out from the messenger. send_linger() does the same for watch/notify registration and watch reconnect requests. Unfortunately cancel_request() doesn't guarantee that after it returns the OSD client would be completely done with the OSD request -- a ref could still be held and the callback (if specified) could still be invoked too. The original motivation for request_reinit() was inability to deal with allocation failures in send_linger() and send_linger_ping(). Switching to using osdc->req_mempool (currently only used by CephFS) respects that and allows us to get rid of request_reinit().

Ilya Dryomov committed 4 years ago

Commit 75dbb681 parent 42226c92 files · +122 −183Browse files

Showing the changes to include/linux/ceph (1 of 2 files). Show all

Unified · Split

include/linux/ceph/osd_client.h+3−0

@@ -287,6 +287,9 @@ struct ceph_osd_linger_request { · 286 unmodified lines
287	rados_watcherrcb_t errcb;	287	rados_watcherrcb_t errcb;
288	void *data;	288	void *data;
289		289
		290	struct ceph_pagelist *request_pl;
		291	struct page **notify_id_pages;
		292
290	struct page ***preply_pages;	293	struct page ***preply_pages;
291	size_t *preply_len;	294	size_t *preply_len;
292	};	295	};