Commit 6c1ea260 authored by Ilya Dryomov's avatar Ilya Dryomov

libceph: make authorizer destruction independent of ceph_auth_client

Starting the kernel client with cephx disabled and then enabling cephx
and restarting userspace daemons can result in a crash:

    [262671.478162] BUG: unable to handle kernel paging request at ffffebe000000000
    [262671.531460] IP: [<ffffffff811cd04a>] kfree+0x5a/0x130
    [262671.584334] PGD 0
    [262671.635847] Oops: 0000 [#1] SMP
    [262672.055841] CPU: 22 PID: 2961272 Comm: kworker/22:2 Not tainted 4.2.0-34-generic #39~14.04.1-Ubuntu
    [262672.162338] Hardware name: Dell Inc. PowerEdge R720/068CDY, BIOS 2.4.3 07/09/2014
    [262672.268937] Workqueue: ceph-msgr con_work [libceph]
    [262672.322290] task: ffff88081c2d0dc0 ti: ffff880149ae8000 task.ti: ffff880149ae8000
    [262672.428330] RIP: 0010:[<ffffffff811cd04a>]  [<ffffffff811cd04a>] kfree+0x5a/0x130
    [262672.535880] RSP: 0018:ffff880149aeba58  EFLAGS: 00010286
    [262672.589486] RAX: 000001e000000000 RBX: 0000000000000012 RCX: ffff8807e7461018
    [262672.695980] RDX: 000077ff80000000 RSI: ffff88081af2be04 RDI: 0000000000000012
    [262672.803668] RBP: ffff880149aeba78 R08: 0000000000000000 R09: 0000000000000000
    [262672.912299] R10: ffffebe000000000 R11: ffff880819a60e78 R12: ffff8800aec8df40
    [262673.021769] R13: ffffffffc035f70f R14: ffff8807e5b138e0 R15: ffff880da9785840
    [262673.131722] FS:  0000000000000000(0000) GS:ffff88081fac0000(0000) knlGS:0000000000000000
    [262673.245377] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [262673.303281] CR2: ffffebe000000000 CR3: 0000000001c0d000 CR4: 00000000001406e0
    [262673.417556] Stack:
    [262673.472943]  ffff880149aeba88 ffff88081af2be04 ffff8800aec8df40 ffff88081af2be04
    [262673.583767]  ffff880149aeba98 ffffffffc035f70f ffff880149aebac8 ffff8800aec8df00
    [262673.694546]  ffff880149aebac8 ffffffffc035c89e ffff8807e5b138e0 ffff8805b047f800
    [262673.805230] Call Trace:
    [262673.859116]  [<ffffffffc035f70f>] ceph_x_destroy_authorizer+0x1f/0x50 [libceph]
    [262673.968705]  [<ffffffffc035c89e>] ceph_auth_destroy_authorizer+0x3e/0x60 [libceph]
    [262674.078852]  [<ffffffffc0352805>] put_osd+0x45/0x80 [libceph]
    [262674.134249]  [<ffffffffc035290e>] remove_osd+0xae/0x140 [libceph]
    [262674.189124]  [<ffffffffc0352aa3>] __reset_osd+0x103/0x150 [libceph]
    [262674.243749]  [<ffffffffc0354703>] kick_requests+0x223/0x460 [libceph]
    [262674.297485]  [<ffffffffc03559e2>] ceph_osdc_handle_map+0x282/0x5e0 [libceph]
    [262674.350813]  [<ffffffffc035022e>] dispatch+0x4e/0x720 [libceph]
    [262674.403312]  [<ffffffffc034bd91>] try_read+0x3d1/0x1090 [libceph]
    [262674.454712]  [<ffffffff810ab7c2>] ? dequeue_entity+0x152/0x690
    [262674.505096]  [<ffffffffc034cb1b>] con_work+0xcb/0x1300 [libceph]
    [262674.555104]  [<ffffffff8108fb3e>] process_one_work+0x14e/0x3d0
    [262674.604072]  [<ffffffff810901ea>] worker_thread+0x11a/0x470
    [262674.652187]  [<ffffffff810900d0>] ? rescuer_thread+0x310/0x310
    [262674.699022]  [<ffffffff810957a2>] kthread+0xd2/0xf0
    [262674.744494]  [<ffffffff810956d0>] ? kthread_create_on_node+0x1c0/0x1c0
    [262674.789543]  [<ffffffff817bd81f>] ret_from_fork+0x3f/0x70
    [262674.834094]  [<ffffffff810956d0>] ? kthread_create_on_node+0x1c0/0x1c0

What happens is the following:

    (1) new MON session is established
    (2) old "none" ac is destroyed
    (3) new "cephx" ac is constructed
    ...
    (4) old OSD session (w/ "none" authorizer) is put
          ceph_auth_destroy_authorizer(ac, osd->o_auth.authorizer)

osd->o_auth.authorizer in the "none" case is just a bare pointer into
ac, which contains a single static copy for all services.  By the time
we get to (4), "none" ac, freed in (2), is long gone.  On top of that,
a new vtable installed in (3) points us at ceph_x_destroy_authorizer(),
so we end up trying to destroy a "none" authorizer with a "cephx"
destructor operating on invalid memory!

To fix this, decouple authorizer destruction from ac and do away with
a single static "none" authorizer by making a copy for each OSD or MDS
session.  Authorizers themselves are independent of ac and so there is
no reason for destroy_authorizer() to be an ac op.  Make it an op on
the authorizer itself by turning ceph_authorizer into a real struct.

Fixes: http://tracker.ceph.com/issues/15447Reported-by: default avatarAlan Zhang <alan.zhang@linux.com>
Signed-off-by: default avatarIlya Dryomov <idryomov@gmail.com>
Reviewed-by: default avatarSage Weil <sage@redhat.com>
parent 02da2d72
...@@ -386,9 +386,7 @@ void ceph_put_mds_session(struct ceph_mds_session *s) ...@@ -386,9 +386,7 @@ void ceph_put_mds_session(struct ceph_mds_session *s)
atomic_read(&s->s_ref), atomic_read(&s->s_ref)-1); atomic_read(&s->s_ref), atomic_read(&s->s_ref)-1);
if (atomic_dec_and_test(&s->s_ref)) { if (atomic_dec_and_test(&s->s_ref)) {
if (s->s_auth.authorizer) if (s->s_auth.authorizer)
ceph_auth_destroy_authorizer( ceph_auth_destroy_authorizer(s->s_auth.authorizer);
s->s_mdsc->fsc->client->monc.auth,
s->s_auth.authorizer);
kfree(s); kfree(s);
} }
} }
...@@ -3900,7 +3898,7 @@ static struct ceph_auth_handshake *get_authorizer(struct ceph_connection *con, ...@@ -3900,7 +3898,7 @@ static struct ceph_auth_handshake *get_authorizer(struct ceph_connection *con,
struct ceph_auth_handshake *auth = &s->s_auth; struct ceph_auth_handshake *auth = &s->s_auth;
if (force_new && auth->authorizer) { if (force_new && auth->authorizer) {
ceph_auth_destroy_authorizer(ac, auth->authorizer); ceph_auth_destroy_authorizer(auth->authorizer);
auth->authorizer = NULL; auth->authorizer = NULL;
} }
if (!auth->authorizer) { if (!auth->authorizer) {
......
...@@ -12,9 +12,12 @@ ...@@ -12,9 +12,12 @@
*/ */
struct ceph_auth_client; struct ceph_auth_client;
struct ceph_authorizer;
struct ceph_msg; struct ceph_msg;
struct ceph_authorizer {
void (*destroy)(struct ceph_authorizer *);
};
struct ceph_auth_handshake { struct ceph_auth_handshake {
struct ceph_authorizer *authorizer; struct ceph_authorizer *authorizer;
void *authorizer_buf; void *authorizer_buf;
...@@ -62,8 +65,6 @@ struct ceph_auth_client_ops { ...@@ -62,8 +65,6 @@ struct ceph_auth_client_ops {
struct ceph_auth_handshake *auth); struct ceph_auth_handshake *auth);
int (*verify_authorizer_reply)(struct ceph_auth_client *ac, int (*verify_authorizer_reply)(struct ceph_auth_client *ac,
struct ceph_authorizer *a, size_t len); struct ceph_authorizer *a, size_t len);
void (*destroy_authorizer)(struct ceph_auth_client *ac,
struct ceph_authorizer *a);
void (*invalidate_authorizer)(struct ceph_auth_client *ac, void (*invalidate_authorizer)(struct ceph_auth_client *ac,
int peer_type); int peer_type);
...@@ -112,8 +113,7 @@ extern int ceph_auth_is_authenticated(struct ceph_auth_client *ac); ...@@ -112,8 +113,7 @@ extern int ceph_auth_is_authenticated(struct ceph_auth_client *ac);
extern int ceph_auth_create_authorizer(struct ceph_auth_client *ac, extern int ceph_auth_create_authorizer(struct ceph_auth_client *ac,
int peer_type, int peer_type,
struct ceph_auth_handshake *auth); struct ceph_auth_handshake *auth);
extern void ceph_auth_destroy_authorizer(struct ceph_auth_client *ac, void ceph_auth_destroy_authorizer(struct ceph_authorizer *a);
struct ceph_authorizer *a);
extern int ceph_auth_update_authorizer(struct ceph_auth_client *ac, extern int ceph_auth_update_authorizer(struct ceph_auth_client *ac,
int peer_type, int peer_type,
struct ceph_auth_handshake *a); struct ceph_auth_handshake *a);
......
...@@ -16,7 +16,6 @@ struct ceph_msg; ...@@ -16,7 +16,6 @@ struct ceph_msg;
struct ceph_snap_context; struct ceph_snap_context;
struct ceph_osd_request; struct ceph_osd_request;
struct ceph_osd_client; struct ceph_osd_client;
struct ceph_authorizer;
/* /*
* completion callback for async writepages * completion callback for async writepages
......
...@@ -293,13 +293,9 @@ int ceph_auth_create_authorizer(struct ceph_auth_client *ac, ...@@ -293,13 +293,9 @@ int ceph_auth_create_authorizer(struct ceph_auth_client *ac,
} }
EXPORT_SYMBOL(ceph_auth_create_authorizer); EXPORT_SYMBOL(ceph_auth_create_authorizer);
void ceph_auth_destroy_authorizer(struct ceph_auth_client *ac, void ceph_auth_destroy_authorizer(struct ceph_authorizer *a)
struct ceph_authorizer *a)
{ {
mutex_lock(&ac->mutex); a->destroy(a);
if (ac->ops && ac->ops->destroy_authorizer)
ac->ops->destroy_authorizer(ac, a);
mutex_unlock(&ac->mutex);
} }
EXPORT_SYMBOL(ceph_auth_destroy_authorizer); EXPORT_SYMBOL(ceph_auth_destroy_authorizer);
......
...@@ -16,7 +16,6 @@ static void reset(struct ceph_auth_client *ac) ...@@ -16,7 +16,6 @@ static void reset(struct ceph_auth_client *ac)
struct ceph_auth_none_info *xi = ac->private; struct ceph_auth_none_info *xi = ac->private;
xi->starting = true; xi->starting = true;
xi->built_authorizer = false;
} }
static void destroy(struct ceph_auth_client *ac) static void destroy(struct ceph_auth_client *ac)
...@@ -39,6 +38,27 @@ static int should_authenticate(struct ceph_auth_client *ac) ...@@ -39,6 +38,27 @@ static int should_authenticate(struct ceph_auth_client *ac)
return xi->starting; return xi->starting;
} }
static int ceph_auth_none_build_authorizer(struct ceph_auth_client *ac,
struct ceph_none_authorizer *au)
{
void *p = au->buf;
void *const end = p + sizeof(au->buf);
int ret;
ceph_encode_8_safe(&p, end, 1, e_range);
ret = ceph_entity_name_encode(ac->name, &p, end);
if (ret < 0)
return ret;
ceph_encode_64_safe(&p, end, ac->global_id, e_range);
au->buf_len = p - (void *)au->buf;
dout("%s built authorizer len %d\n", __func__, au->buf_len);
return 0;
e_range:
return -ERANGE;
}
static int build_request(struct ceph_auth_client *ac, void *buf, void *end) static int build_request(struct ceph_auth_client *ac, void *buf, void *end)
{ {
return 0; return 0;
...@@ -57,32 +77,32 @@ static int handle_reply(struct ceph_auth_client *ac, int result, ...@@ -57,32 +77,32 @@ static int handle_reply(struct ceph_auth_client *ac, int result,
return result; return result;
} }
static void ceph_auth_none_destroy_authorizer(struct ceph_authorizer *a)
{
kfree(a);
}
/* /*
* build an 'authorizer' with our entity_name and global_id. we can * build an 'authorizer' with our entity_name and global_id. it is
* reuse a single static copy since it is identical for all services * identical for all services we connect to.
* we connect to.
*/ */
static int ceph_auth_none_create_authorizer( static int ceph_auth_none_create_authorizer(
struct ceph_auth_client *ac, int peer_type, struct ceph_auth_client *ac, int peer_type,
struct ceph_auth_handshake *auth) struct ceph_auth_handshake *auth)
{ {
struct ceph_auth_none_info *ai = ac->private; struct ceph_none_authorizer *au;
struct ceph_none_authorizer *au = &ai->au;
void *p, *end;
int ret; int ret;
if (!ai->built_authorizer) { au = kmalloc(sizeof(*au), GFP_NOFS);
p = au->buf; if (!au)
end = p + sizeof(au->buf); return -ENOMEM;
ceph_encode_8(&p, 1);
ret = ceph_entity_name_encode(ac->name, &p, end - 8); au->base.destroy = ceph_auth_none_destroy_authorizer;
if (ret < 0)
goto bad; ret = ceph_auth_none_build_authorizer(ac, au);
ceph_decode_need(&p, end, sizeof(u64), bad2); if (ret) {
ceph_encode_64(&p, ac->global_id); kfree(au);
au->buf_len = p - (void *)au->buf; return ret;
ai->built_authorizer = true;
dout("built authorizer len %d\n", au->buf_len);
} }
auth->authorizer = (struct ceph_authorizer *) au; auth->authorizer = (struct ceph_authorizer *) au;
...@@ -92,17 +112,6 @@ static int ceph_auth_none_create_authorizer( ...@@ -92,17 +112,6 @@ static int ceph_auth_none_create_authorizer(
auth->authorizer_reply_buf_len = sizeof (au->reply_buf); auth->authorizer_reply_buf_len = sizeof (au->reply_buf);
return 0; return 0;
bad2:
ret = -ERANGE;
bad:
return ret;
}
static void ceph_auth_none_destroy_authorizer(struct ceph_auth_client *ac,
struct ceph_authorizer *a)
{
/* nothing to do */
} }
static const struct ceph_auth_client_ops ceph_auth_none_ops = { static const struct ceph_auth_client_ops ceph_auth_none_ops = {
...@@ -114,7 +123,6 @@ static const struct ceph_auth_client_ops ceph_auth_none_ops = { ...@@ -114,7 +123,6 @@ static const struct ceph_auth_client_ops ceph_auth_none_ops = {
.build_request = build_request, .build_request = build_request,
.handle_reply = handle_reply, .handle_reply = handle_reply,
.create_authorizer = ceph_auth_none_create_authorizer, .create_authorizer = ceph_auth_none_create_authorizer,
.destroy_authorizer = ceph_auth_none_destroy_authorizer,
}; };
int ceph_auth_none_init(struct ceph_auth_client *ac) int ceph_auth_none_init(struct ceph_auth_client *ac)
...@@ -127,7 +135,6 @@ int ceph_auth_none_init(struct ceph_auth_client *ac) ...@@ -127,7 +135,6 @@ int ceph_auth_none_init(struct ceph_auth_client *ac)
return -ENOMEM; return -ENOMEM;
xi->starting = true; xi->starting = true;
xi->built_authorizer = false;
ac->protocol = CEPH_AUTH_NONE; ac->protocol = CEPH_AUTH_NONE;
ac->private = xi; ac->private = xi;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
*/ */
struct ceph_none_authorizer { struct ceph_none_authorizer {
struct ceph_authorizer base;
char buf[128]; char buf[128];
int buf_len; int buf_len;
char reply_buf[0]; char reply_buf[0];
...@@ -19,8 +20,6 @@ struct ceph_none_authorizer { ...@@ -19,8 +20,6 @@ struct ceph_none_authorizer {
struct ceph_auth_none_info { struct ceph_auth_none_info {
bool starting; bool starting;
bool built_authorizer;
struct ceph_none_authorizer au; /* we only need one; it's static */
}; };
int ceph_auth_none_init(struct ceph_auth_client *ac); int ceph_auth_none_init(struct ceph_auth_client *ac);
......
...@@ -565,6 +565,14 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result, ...@@ -565,6 +565,14 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result,
return -EAGAIN; return -EAGAIN;
} }
static void ceph_x_destroy_authorizer(struct ceph_authorizer *a)
{
struct ceph_x_authorizer *au = (void *)a;
ceph_x_authorizer_cleanup(au);
kfree(au);
}
static int ceph_x_create_authorizer( static int ceph_x_create_authorizer(
struct ceph_auth_client *ac, int peer_type, struct ceph_auth_client *ac, int peer_type,
struct ceph_auth_handshake *auth) struct ceph_auth_handshake *auth)
...@@ -581,6 +589,8 @@ static int ceph_x_create_authorizer( ...@@ -581,6 +589,8 @@ static int ceph_x_create_authorizer(
if (!au) if (!au)
return -ENOMEM; return -ENOMEM;
au->base.destroy = ceph_x_destroy_authorizer;
ret = ceph_x_build_authorizer(ac, th, au); ret = ceph_x_build_authorizer(ac, th, au);
if (ret) { if (ret) {
kfree(au); kfree(au);
...@@ -643,16 +653,6 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac, ...@@ -643,16 +653,6 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac,
return ret; return ret;
} }
static void ceph_x_destroy_authorizer(struct ceph_auth_client *ac,
struct ceph_authorizer *a)
{
struct ceph_x_authorizer *au = (void *)a;
ceph_x_authorizer_cleanup(au);
kfree(au);
}
static void ceph_x_reset(struct ceph_auth_client *ac) static void ceph_x_reset(struct ceph_auth_client *ac)
{ {
struct ceph_x_info *xi = ac->private; struct ceph_x_info *xi = ac->private;
...@@ -770,7 +770,6 @@ static const struct ceph_auth_client_ops ceph_x_ops = { ...@@ -770,7 +770,6 @@ static const struct ceph_auth_client_ops ceph_x_ops = {
.create_authorizer = ceph_x_create_authorizer, .create_authorizer = ceph_x_create_authorizer,
.update_authorizer = ceph_x_update_authorizer, .update_authorizer = ceph_x_update_authorizer,
.verify_authorizer_reply = ceph_x_verify_authorizer_reply, .verify_authorizer_reply = ceph_x_verify_authorizer_reply,
.destroy_authorizer = ceph_x_destroy_authorizer,
.invalidate_authorizer = ceph_x_invalidate_authorizer, .invalidate_authorizer = ceph_x_invalidate_authorizer,
.reset = ceph_x_reset, .reset = ceph_x_reset,
.destroy = ceph_x_destroy, .destroy = ceph_x_destroy,
......
...@@ -26,6 +26,7 @@ struct ceph_x_ticket_handler { ...@@ -26,6 +26,7 @@ struct ceph_x_ticket_handler {
struct ceph_x_authorizer { struct ceph_x_authorizer {
struct ceph_authorizer base;
struct ceph_crypto_key session_key; struct ceph_crypto_key session_key;
struct ceph_buffer *buf; struct ceph_buffer *buf;
unsigned int service; unsigned int service;
......
...@@ -1087,10 +1087,8 @@ static void put_osd(struct ceph_osd *osd) ...@@ -1087,10 +1087,8 @@ static void put_osd(struct ceph_osd *osd)
dout("put_osd %p %d -> %d\n", osd, atomic_read(&osd->o_ref), dout("put_osd %p %d -> %d\n", osd, atomic_read(&osd->o_ref),
atomic_read(&osd->o_ref) - 1); atomic_read(&osd->o_ref) - 1);
if (atomic_dec_and_test(&osd->o_ref)) { if (atomic_dec_and_test(&osd->o_ref)) {
struct ceph_auth_client *ac = osd->o_osdc->client->monc.auth;
if (osd->o_auth.authorizer) if (osd->o_auth.authorizer)
ceph_auth_destroy_authorizer(ac, osd->o_auth.authorizer); ceph_auth_destroy_authorizer(osd->o_auth.authorizer);
kfree(osd); kfree(osd);
} }
} }
...@@ -2984,7 +2982,7 @@ static struct ceph_auth_handshake *get_authorizer(struct ceph_connection *con, ...@@ -2984,7 +2982,7 @@ static struct ceph_auth_handshake *get_authorizer(struct ceph_connection *con,
struct ceph_auth_handshake *auth = &o->o_auth; struct ceph_auth_handshake *auth = &o->o_auth;
if (force_new && auth->authorizer) { if (force_new && auth->authorizer) {
ceph_auth_destroy_authorizer(ac, auth->authorizer); ceph_auth_destroy_authorizer(auth->authorizer);
auth->authorizer = NULL; auth->authorizer = NULL;
} }
if (!auth->authorizer) { if (!auth->authorizer) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment