From 58d1362b50dc87ebf18cd137e7a879fd99b7e721 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Tue, 12 Aug 2014 11:22:07 +0400 Subject: [PATCH 1/5] rbd: avoid format-security warning inside alloc_workqueue() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drivers/block/rbd.c: In function ‘rbd_dev_device_setup’: drivers/block/rbd.c:5090:19: warning: format not a string literal and no format arguments [-Wformat-security] Reported-by: kbuild test robot Signed-off-by: Ilya Dryomov --- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 623c84145b79..2cd01ca1b5b4 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -5087,7 +5087,7 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev) set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE); set_disk_ro(rbd_dev->disk, rbd_dev->mapping.read_only); - rbd_dev->rq_wq = alloc_workqueue(rbd_dev->disk->disk_name, 0, 0); + rbd_dev->rq_wq = alloc_workqueue("%s", 0, 0, rbd_dev->disk->disk_name); if (!rbd_dev->rq_wq) goto err_out_mapping; From 255939e783d8f45f8c58487dfc18957c44ea9871 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 13 Aug 2014 20:49:52 -0700 Subject: [PATCH 2/5] rbd: fix error return code in rbd_dev_device_setup() Fix to return -ENOMEM from the workqueue alloc error handling case instead of 0, as done elsewhere in this function. Reviewed-by: Alex Elder Signed-off-by: Wei Yongjun --- drivers/block/rbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 2cd01ca1b5b4..4b97baf8afa3 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -5088,8 +5088,10 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev) set_disk_ro(rbd_dev->disk, rbd_dev->mapping.read_only); rbd_dev->rq_wq = alloc_workqueue("%s", 0, 0, rbd_dev->disk->disk_name); - if (!rbd_dev->rq_wq) + if (!rbd_dev->rq_wq) { + ret = -ENOMEM; goto err_out_mapping; + } ret = rbd_bus_add_dev(rbd_dev); if (ret) From 73c3d4812b4c755efeca0140f606f83772a39ce4 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 4 Aug 2014 07:01:54 -0700 Subject: [PATCH 3/5] libceph: gracefully handle large reply messages from the mon We preallocate a few of the message types we get back from the mon. If we get a larger message than we are expecting, fall back to trying to allocate a new one instead of blindly using the one we have. CC: stable@vger.kernel.org Signed-off-by: Sage Weil Reviewed-by: Ilya Dryomov --- net/ceph/mon_client.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c index 067d3af2eaf6..61fcfc304f68 100644 --- a/net/ceph/mon_client.c +++ b/net/ceph/mon_client.c @@ -1181,7 +1181,15 @@ static struct ceph_msg *mon_alloc_msg(struct ceph_connection *con, if (!m) { pr_info("alloc_msg unknown type %d\n", type); *skip = 1; + } else if (front_len > m->front_alloc_len) { + pr_warning("mon_alloc_msg front %d > prealloc %d (%u#%llu)\n", + front_len, m->front_alloc_len, + (unsigned int)con->peer_name.type, + le64_to_cpu(con->peer_name.num)); + ceph_msg_put(m); + m = ceph_msg_new(type, front_len, GFP_NOFS, false); } + return m; } From 597cda357716a3cf8d994cb11927af917c8d71fa Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 8 Sep 2014 17:25:34 +0400 Subject: [PATCH 4/5] libceph: add process_one_ticket() helper Add a helper for processing individual cephx auth tickets. Needed for the next commit, which deals with allocating ticket buffers. (Most of the diff here is whitespace - view with git diff -b). Cc: stable@vger.kernel.org Signed-off-by: Ilya Dryomov Reviewed-by: Sage Weil --- net/ceph/auth_x.c | 228 +++++++++++++++++++++++++--------------------- 1 file changed, 124 insertions(+), 104 deletions(-) diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c index 96238ba95f2b..0eb146dce1aa 100644 --- a/net/ceph/auth_x.c +++ b/net/ceph/auth_x.c @@ -129,17 +129,131 @@ static void remove_ticket_handler(struct ceph_auth_client *ac, kfree(th); } +static int process_one_ticket(struct ceph_auth_client *ac, + struct ceph_crypto_key *secret, + void **p, void *end, + void *dbuf, void *ticket_buf) +{ + struct ceph_x_info *xi = ac->private; + int type; + u8 tkt_struct_v, blob_struct_v; + struct ceph_x_ticket_handler *th; + void *dp, *dend; + int dlen; + char is_enc; + struct timespec validity; + struct ceph_crypto_key old_key; + void *tp, *tpend; + struct ceph_timespec new_validity; + struct ceph_crypto_key new_session_key; + struct ceph_buffer *new_ticket_blob; + unsigned long new_expires, new_renew_after; + u64 new_secret_id; + int ret; + + ceph_decode_need(p, end, sizeof(u32) + 1, bad); + + type = ceph_decode_32(p); + dout(" ticket type %d %s\n", type, ceph_entity_type_name(type)); + + tkt_struct_v = ceph_decode_8(p); + if (tkt_struct_v != 1) + goto bad; + + th = get_ticket_handler(ac, type); + if (IS_ERR(th)) { + ret = PTR_ERR(th); + goto out; + } + + /* blob for me */ + dlen = ceph_x_decrypt(secret, p, end, dbuf, + TEMP_TICKET_BUF_LEN); + if (dlen <= 0) { + ret = dlen; + goto out; + } + dout(" decrypted %d bytes\n", dlen); + dp = dbuf; + dend = dp + dlen; + + tkt_struct_v = ceph_decode_8(&dp); + if (tkt_struct_v != 1) + goto bad; + + memcpy(&old_key, &th->session_key, sizeof(old_key)); + ret = ceph_crypto_key_decode(&new_session_key, &dp, dend); + if (ret) + goto out; + + ceph_decode_copy(&dp, &new_validity, sizeof(new_validity)); + ceph_decode_timespec(&validity, &new_validity); + new_expires = get_seconds() + validity.tv_sec; + new_renew_after = new_expires - (validity.tv_sec / 4); + dout(" expires=%lu renew_after=%lu\n", new_expires, + new_renew_after); + + /* ticket blob for service */ + ceph_decode_8_safe(p, end, is_enc, bad); + tp = ticket_buf; + if (is_enc) { + /* encrypted */ + dout(" encrypted ticket\n"); + dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf, + TEMP_TICKET_BUF_LEN); + if (dlen < 0) { + ret = dlen; + goto out; + } + dlen = ceph_decode_32(&tp); + } else { + /* unencrypted */ + ceph_decode_32_safe(p, end, dlen, bad); + ceph_decode_need(p, end, dlen, bad); + ceph_decode_copy(p, ticket_buf, dlen); + } + tpend = tp + dlen; + dout(" ticket blob is %d bytes\n", dlen); + ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad); + blob_struct_v = ceph_decode_8(&tp); + new_secret_id = ceph_decode_64(&tp); + ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend); + if (ret) + goto out; + + /* all is well, update our ticket */ + ceph_crypto_key_destroy(&th->session_key); + if (th->ticket_blob) + ceph_buffer_put(th->ticket_blob); + th->session_key = new_session_key; + th->ticket_blob = new_ticket_blob; + th->validity = new_validity; + th->secret_id = new_secret_id; + th->expires = new_expires; + th->renew_after = new_renew_after; + dout(" got ticket service %d (%s) secret_id %lld len %d\n", + type, ceph_entity_type_name(type), th->secret_id, + (int)th->ticket_blob->vec.iov_len); + xi->have_keys |= th->service; + +out: + return ret; + +bad: + ret = -EINVAL; + goto out; +} + static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac, struct ceph_crypto_key *secret, void *buf, void *end) { - struct ceph_x_info *xi = ac->private; - int num; void *p = buf; - int ret; char *dbuf; char *ticket_buf; u8 reply_struct_v; + u32 num; + int ret; dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS); if (!dbuf) @@ -150,112 +264,18 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac, if (!ticket_buf) goto out_dbuf; - ceph_decode_need(&p, end, 1 + sizeof(u32), bad); - reply_struct_v = ceph_decode_8(&p); + ceph_decode_8_safe(&p, end, reply_struct_v, bad); if (reply_struct_v != 1) - goto bad; - num = ceph_decode_32(&p); + return -EINVAL; + + ceph_decode_32_safe(&p, end, num, bad); dout("%d tickets\n", num); + while (num--) { - int type; - u8 tkt_struct_v, blob_struct_v; - struct ceph_x_ticket_handler *th; - void *dp, *dend; - int dlen; - char is_enc; - struct timespec validity; - struct ceph_crypto_key old_key; - void *tp, *tpend; - struct ceph_timespec new_validity; - struct ceph_crypto_key new_session_key; - struct ceph_buffer *new_ticket_blob; - unsigned long new_expires, new_renew_after; - u64 new_secret_id; - - ceph_decode_need(&p, end, sizeof(u32) + 1, bad); - - type = ceph_decode_32(&p); - dout(" ticket type %d %s\n", type, ceph_entity_type_name(type)); - - tkt_struct_v = ceph_decode_8(&p); - if (tkt_struct_v != 1) - goto bad; - - th = get_ticket_handler(ac, type); - if (IS_ERR(th)) { - ret = PTR_ERR(th); - goto out; - } - - /* blob for me */ - dlen = ceph_x_decrypt(secret, &p, end, dbuf, - TEMP_TICKET_BUF_LEN); - if (dlen <= 0) { - ret = dlen; - goto out; - } - dout(" decrypted %d bytes\n", dlen); - dend = dbuf + dlen; - dp = dbuf; - - tkt_struct_v = ceph_decode_8(&dp); - if (tkt_struct_v != 1) - goto bad; - - memcpy(&old_key, &th->session_key, sizeof(old_key)); - ret = ceph_crypto_key_decode(&new_session_key, &dp, dend); + ret = process_one_ticket(ac, secret, &p, end, + dbuf, ticket_buf); if (ret) goto out; - - ceph_decode_copy(&dp, &new_validity, sizeof(new_validity)); - ceph_decode_timespec(&validity, &new_validity); - new_expires = get_seconds() + validity.tv_sec; - new_renew_after = new_expires - (validity.tv_sec / 4); - dout(" expires=%lu renew_after=%lu\n", new_expires, - new_renew_after); - - /* ticket blob for service */ - ceph_decode_8_safe(&p, end, is_enc, bad); - tp = ticket_buf; - if (is_enc) { - /* encrypted */ - dout(" encrypted ticket\n"); - dlen = ceph_x_decrypt(&old_key, &p, end, ticket_buf, - TEMP_TICKET_BUF_LEN); - if (dlen < 0) { - ret = dlen; - goto out; - } - dlen = ceph_decode_32(&tp); - } else { - /* unencrypted */ - ceph_decode_32_safe(&p, end, dlen, bad); - ceph_decode_need(&p, end, dlen, bad); - ceph_decode_copy(&p, ticket_buf, dlen); - } - tpend = tp + dlen; - dout(" ticket blob is %d bytes\n", dlen); - ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad); - blob_struct_v = ceph_decode_8(&tp); - new_secret_id = ceph_decode_64(&tp); - ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend); - if (ret) - goto out; - - /* all is well, update our ticket */ - ceph_crypto_key_destroy(&th->session_key); - if (th->ticket_blob) - ceph_buffer_put(th->ticket_blob); - th->session_key = new_session_key; - th->ticket_blob = new_ticket_blob; - th->validity = new_validity; - th->secret_id = new_secret_id; - th->expires = new_expires; - th->renew_after = new_renew_after; - dout(" got ticket service %d (%s) secret_id %lld len %d\n", - type, ceph_entity_type_name(type), th->secret_id, - (int)th->ticket_blob->vec.iov_len); - xi->have_keys |= th->service; } ret = 0; From c27a3e4d667fdcad3db7b104f75659478e0c68d8 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Tue, 9 Sep 2014 19:39:15 +0400 Subject: [PATCH 5/5] libceph: do not hard code max auth ticket len We hard code cephx auth ticket buffer size to 256 bytes. This isn't enough for any moderate setups and, in case tickets themselves are not encrypted, leads to buffer overflows (ceph_x_decrypt() errors out, but ceph_decode_copy() doesn't - it's just a memcpy() wrapper). Since the buffer is allocated dynamically anyway, allocated it a bit later, at the point where we know how much is going to be needed. Fixes: http://tracker.ceph.com/issues/8979 Cc: stable@vger.kernel.org Signed-off-by: Ilya Dryomov Reviewed-by: Sage Weil --- net/ceph/auth_x.c | 64 +++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c index 0eb146dce1aa..de6662b14e1f 100644 --- a/net/ceph/auth_x.c +++ b/net/ceph/auth_x.c @@ -13,8 +13,6 @@ #include "auth_x.h" #include "auth_x_protocol.h" -#define TEMP_TICKET_BUF_LEN 256 - static void ceph_x_validate_tickets(struct ceph_auth_client *ac, int *pneed); static int ceph_x_is_authenticated(struct ceph_auth_client *ac) @@ -64,7 +62,7 @@ static int ceph_x_encrypt(struct ceph_crypto_key *secret, } static int ceph_x_decrypt(struct ceph_crypto_key *secret, - void **p, void *end, void *obuf, size_t olen) + void **p, void *end, void **obuf, size_t olen) { struct ceph_x_encrypt_header head; size_t head_len = sizeof(head); @@ -75,8 +73,14 @@ static int ceph_x_decrypt(struct ceph_crypto_key *secret, return -EINVAL; dout("ceph_x_decrypt len %d\n", len); - ret = ceph_decrypt2(secret, &head, &head_len, obuf, &olen, - *p, len); + if (*obuf == NULL) { + *obuf = kmalloc(len, GFP_NOFS); + if (!*obuf) + return -ENOMEM; + olen = len; + } + + ret = ceph_decrypt2(secret, &head, &head_len, *obuf, &olen, *p, len); if (ret) return ret; if (head.struct_v != 1 || le64_to_cpu(head.magic) != CEPHX_ENC_MAGIC) @@ -131,18 +135,19 @@ static void remove_ticket_handler(struct ceph_auth_client *ac, static int process_one_ticket(struct ceph_auth_client *ac, struct ceph_crypto_key *secret, - void **p, void *end, - void *dbuf, void *ticket_buf) + void **p, void *end) { struct ceph_x_info *xi = ac->private; int type; u8 tkt_struct_v, blob_struct_v; struct ceph_x_ticket_handler *th; + void *dbuf = NULL; void *dp, *dend; int dlen; char is_enc; struct timespec validity; struct ceph_crypto_key old_key; + void *ticket_buf = NULL; void *tp, *tpend; struct ceph_timespec new_validity; struct ceph_crypto_key new_session_key; @@ -167,8 +172,7 @@ static int process_one_ticket(struct ceph_auth_client *ac, } /* blob for me */ - dlen = ceph_x_decrypt(secret, p, end, dbuf, - TEMP_TICKET_BUF_LEN); + dlen = ceph_x_decrypt(secret, p, end, &dbuf, 0); if (dlen <= 0) { ret = dlen; goto out; @@ -195,20 +199,25 @@ static int process_one_ticket(struct ceph_auth_client *ac, /* ticket blob for service */ ceph_decode_8_safe(p, end, is_enc, bad); - tp = ticket_buf; if (is_enc) { /* encrypted */ dout(" encrypted ticket\n"); - dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf, - TEMP_TICKET_BUF_LEN); + dlen = ceph_x_decrypt(&old_key, p, end, &ticket_buf, 0); if (dlen < 0) { ret = dlen; goto out; } + tp = ticket_buf; dlen = ceph_decode_32(&tp); } else { /* unencrypted */ ceph_decode_32_safe(p, end, dlen, bad); + ticket_buf = kmalloc(dlen, GFP_NOFS); + if (!ticket_buf) { + ret = -ENOMEM; + goto out; + } + tp = ticket_buf; ceph_decode_need(p, end, dlen, bad); ceph_decode_copy(p, ticket_buf, dlen); } @@ -237,6 +246,8 @@ static int process_one_ticket(struct ceph_auth_client *ac, xi->have_keys |= th->service; out: + kfree(ticket_buf); + kfree(dbuf); return ret; bad: @@ -249,21 +260,10 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac, void *buf, void *end) { void *p = buf; - char *dbuf; - char *ticket_buf; u8 reply_struct_v; u32 num; int ret; - dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS); - if (!dbuf) - return -ENOMEM; - - ret = -ENOMEM; - ticket_buf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS); - if (!ticket_buf) - goto out_dbuf; - ceph_decode_8_safe(&p, end, reply_struct_v, bad); if (reply_struct_v != 1) return -EINVAL; @@ -272,22 +272,15 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac, dout("%d tickets\n", num); while (num--) { - ret = process_one_ticket(ac, secret, &p, end, - dbuf, ticket_buf); + ret = process_one_ticket(ac, secret, &p, end); if (ret) - goto out; + return ret; } - ret = 0; -out: - kfree(ticket_buf); -out_dbuf: - kfree(dbuf); - return ret; + return 0; bad: - ret = -EINVAL; - goto out; + return -EINVAL; } static int ceph_x_build_authorizer(struct ceph_auth_client *ac, @@ -603,13 +596,14 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac, struct ceph_x_ticket_handler *th; int ret = 0; struct ceph_x_authorize_reply reply; + void *preply = &reply; void *p = au->reply_buf; void *end = p + sizeof(au->reply_buf); th = get_ticket_handler(ac, au->service); if (IS_ERR(th)) return PTR_ERR(th); - ret = ceph_x_decrypt(&th->session_key, &p, end, &reply, sizeof(reply)); + ret = ceph_x_decrypt(&th->session_key, &p, end, &preply, sizeof(reply)); if (ret < 0) return ret; if (ret != sizeof(reply))