From 27171ae6a0fdc75571e5bf3d0961631a1e4fb765 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 1 Jun 2021 09:40:25 -0400 Subject: ceph: must hold snap_rwsem when filling inode for async create ...and add a lockdep assertion for it to ceph_fill_inode(). Cc: stable@vger.kernel.org # v5.7+ Fixes: 9a8d03ca2e2c3 ("ceph: attempt to do async create when possible") Signed-off-by: Jeff Layton Reviewed-by: Ilya Dryomov Signed-off-by: Ilya Dryomov --- fs/ceph/file.c | 3 +++ fs/ceph/inode.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 77fc037d5beb..4dc96885acd0 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -578,6 +578,7 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, struct ceph_inode_info *ci = ceph_inode(dir); struct inode *inode; struct timespec64 now; + struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb); struct ceph_vino vino = { .ino = req->r_deleg_ino, .snap = CEPH_NOSNAP }; @@ -615,8 +616,10 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, ceph_file_layout_to_legacy(lo, &in.layout); + down_read(&mdsc->snap_rwsem); ret = ceph_fill_inode(inode, NULL, &iinfo, NULL, req->r_session, req->r_fmode, NULL); + up_read(&mdsc->snap_rwsem); if (ret) { dout("%s failed to fill inode: %d\n", __func__, ret); ceph_dir_clear_complete(dir); diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index e1c63adb196d..df0c8a724609 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -777,6 +777,8 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page, umode_t mode = le32_to_cpu(info->mode); dev_t rdev = le32_to_cpu(info->rdev); + lockdep_assert_held(&mdsc->snap_rwsem); + dout("%s %p ino %llx.%llx v %llu had %llu\n", __func__, inode, ceph_vinop(inode), le64_to_cpu(info->version), ci->i_version); -- cgit 1.4.1 From 7a971e2c0767b6fc9a77c4108eceff0509c61cdb Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 2 Jun 2021 12:46:07 -0400 Subject: ceph: fix error handling in ceph_atomic_open and ceph_lookup Commit aa60cfc3f7ee broke the error handling in these functions such that they don't handle non-ENOENT errors from ceph_mdsc_do_request properly. Move the checking of -ENOENT out of ceph_handle_snapdir and into the callers, and if we get a different error, return it immediately. Fixes: aa60cfc3f7ee ("ceph: don't use d_add in ceph_handle_snapdir") Signed-off-by: Jeff Layton Reviewed-by: Ilya Dryomov Signed-off-by: Ilya Dryomov --- fs/ceph/dir.c | 22 ++++++++++++---------- fs/ceph/file.c | 14 ++++++++------ fs/ceph/super.h | 2 +- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index 5624fae7a603..9ba79b6531fb 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -668,14 +668,13 @@ out: * Handle lookups for the hidden .snap directory. */ struct dentry *ceph_handle_snapdir(struct ceph_mds_request *req, - struct dentry *dentry, int err) + struct dentry *dentry) { struct ceph_fs_client *fsc = ceph_sb_to_client(dentry->d_sb); struct inode *parent = d_inode(dentry->d_parent); /* we hold i_mutex */ /* .snap dir? */ - if (err == -ENOENT && - ceph_snap(parent) == CEPH_NOSNAP && + if (ceph_snap(parent) == CEPH_NOSNAP && strcmp(dentry->d_name.name, fsc->mount_options->snapdir_name) == 0) { struct dentry *res; struct inode *inode = ceph_get_snapdir(parent); @@ -742,7 +741,6 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry, struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb); struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb); struct ceph_mds_request *req; - struct dentry *res; int op; int mask; int err; @@ -793,12 +791,16 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry, req->r_parent = dir; set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags); err = ceph_mdsc_do_request(mdsc, NULL, req); - res = ceph_handle_snapdir(req, dentry, err); - if (IS_ERR(res)) { - err = PTR_ERR(res); - } else { - dentry = res; - err = 0; + if (err == -ENOENT) { + struct dentry *res; + + res = ceph_handle_snapdir(req, dentry); + if (IS_ERR(res)) { + err = PTR_ERR(res); + } else { + dentry = res; + err = 0; + } } dentry = ceph_finish_lookup(req, dentry, err); ceph_mdsc_put_request(req); /* will dput(dentry) */ diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 4dc96885acd0..d51af3698032 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -742,14 +742,16 @@ retry: err = ceph_mdsc_do_request(mdsc, (flags & (O_CREAT|O_TRUNC)) ? dir : NULL, req); - dentry = ceph_handle_snapdir(req, dentry, err); - if (IS_ERR(dentry)) { - err = PTR_ERR(dentry); - goto out_req; + if (err == -ENOENT) { + dentry = ceph_handle_snapdir(req, dentry); + if (IS_ERR(dentry)) { + err = PTR_ERR(dentry); + goto out_req; + } + err = 0; } - err = 0; - if ((flags & O_CREAT) && !req->r_reply_info.head->is_dentry) + if (!err && (flags & O_CREAT) && !req->r_reply_info.head->is_dentry) err = ceph_handle_notrace_create(dir, dentry); if (d_in_lookup(dentry)) { diff --git a/fs/ceph/super.h b/fs/ceph/super.h index db80d89556b1..839e6b0239ee 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -1218,7 +1218,7 @@ extern const struct dentry_operations ceph_dentry_ops; extern loff_t ceph_make_fpos(unsigned high, unsigned off, bool hash_order); extern int ceph_handle_notrace_create(struct inode *dir, struct dentry *dentry); extern struct dentry *ceph_handle_snapdir(struct ceph_mds_request *req, - struct dentry *dentry, int err); + struct dentry *dentry); extern struct dentry *ceph_finish_lookup(struct ceph_mds_request *req, struct dentry *dentry, int err); -- cgit 1.4.1 From 3c0d0894320cc517fda657c69939cd0313d0b4e2 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 21 Jun 2021 11:53:38 +0200 Subject: libceph: don't pass result into ac->ops->handle_reply() There is no result to pass in msgr2 case because authentication failures are reported through auth_bad_method frame and in MAuth case an error is returned immediately. Signed-off-by: Ilya Dryomov Reviewed-by: Jeff Layton --- include/linux/ceph/auth.h | 2 +- net/ceph/auth.c | 15 ++++++++++----- net/ceph/auth_none.c | 4 ++-- net/ceph/auth_x.c | 6 ++---- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h index 71b5d481c653..39425e2f7cb2 100644 --- a/include/linux/ceph/auth.h +++ b/include/linux/ceph/auth.h @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { * another request. */ int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); - int (*handle_reply)(struct ceph_auth_client *ac, int result, + int (*handle_reply)(struct ceph_auth_client *ac, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len); diff --git a/net/ceph/auth.c b/net/ceph/auth.c index de407e8feb97..3a9d44eee941 100644 --- a/net/ceph/auth.c +++ b/net/ceph/auth.c @@ -260,14 +260,19 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, ac->negotiating = false; } - ret = ac->ops->handle_reply(ac, result, payload, payload_end, + if (result) { + pr_err("auth protocol '%s' mauth authentication failed: %d\n", + ceph_auth_proto_name(ac->protocol), result); + ret = result; + goto out; + } + + ret = ac->ops->handle_reply(ac, payload, payload_end, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) { ret = build_request(ac, true, reply_buf, reply_len); goto out; } else if (ret) { - pr_err("auth protocol '%s' mauth authentication failed: %d\n", - ceph_auth_proto_name(ac->protocol), result); goto out; } @@ -480,7 +485,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, reply, reply + reply_len, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) ret = build_request(ac, false, buf, buf_len); @@ -498,7 +503,7 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, reply, reply + reply_len, session_key, session_key_len, con_secret, con_secret_len); if (!ret) diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c index 70e86e462250..aab490a111eb 100644 --- a/net/ceph/auth_none.c +++ b/net/ceph/auth_none.c @@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end) * the generic auth code decode the global_id, and we carry no actual * authenticate state, so nothing happens here. */ -static int handle_reply(struct ceph_auth_client *ac, int result, +static int handle_reply(struct ceph_auth_client *ac, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -77,7 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, int result, struct ceph_auth_none_info *xi = ac->private; xi->starting = false; - return result; + return 0; } static void ceph_auth_none_destroy_authorizer(struct ceph_authorizer *a) diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c index 79641c4afee9..cab99c5581b0 100644 --- a/net/ceph/auth_x.c +++ b/net/ceph/auth_x.c @@ -661,7 +661,7 @@ e_inval: return -EINVAL; } -static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result, +static int ceph_x_handle_reply(struct ceph_auth_client *ac, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -669,13 +669,11 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result, struct ceph_x_info *xi = ac->private; struct ceph_x_ticket_handler *th; int len = end - buf; + int result; void *p; int op; int ret; - if (result) - return result; /* XXX hmm? */ - if (xi->starting) { /* it's a hello */ struct ceph_x_server_challenge *sc = buf; -- cgit 1.4.1 From 03af4c7bad8ca59143bca488b90b3775d10d7f94 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 21 Jun 2021 12:17:40 +0200 Subject: libceph: set global_id as soon as we get an auth ticket Commit 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") delayed the setting of global_id too much. It is set only after all tickets are received, but in pre-nautilus clusters an auth ticket and the service tickets are obtained in separate steps (for a total of three MAuth replies). When the service tickets are requested, global_id is used to build an authorizer; if global_id is still 0 we never get them and fail to establish the session. Moving the setting of global_id into protocol implementations. This way global_id can be set exactly when an auth ticket is received, not sooner nor later. Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") Signed-off-by: Ilya Dryomov Reviewed-by: Jeff Layton --- include/linux/ceph/auth.h | 4 +++- net/ceph/auth.c | 13 +++++-------- net/ceph/auth_none.c | 3 ++- net/ceph/auth_x.c | 11 ++++++----- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h index 39425e2f7cb2..6b138fa97db8 100644 --- a/include/linux/ceph/auth.h +++ b/include/linux/ceph/auth.h @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { * another request. */ int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); - int (*handle_reply)(struct ceph_auth_client *ac, + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len); @@ -104,6 +104,8 @@ struct ceph_auth_client { struct mutex mutex; }; +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); + struct ceph_auth_client *ceph_auth_init(const char *name, const struct ceph_crypto_key *key, const int *con_modes); diff --git a/net/ceph/auth.c b/net/ceph/auth.c index 3a9d44eee941..d2b268a1838e 100644 --- a/net/ceph/auth.c +++ b/net/ceph/auth.c @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) } } -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) { dout("%s global_id %llu\n", __func__, global_id); @@ -267,7 +267,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, goto out; } - ret = ac->ops->handle_reply(ac, payload, payload_end, + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) { ret = build_request(ac, true, reply_buf, reply_len); @@ -276,8 +276,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, goto out; } - set_global_id(ac, global_id); - out: mutex_unlock(&ac->mutex); return ret; @@ -485,7 +483,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) ret = build_request(ac, false, buf, buf_len); @@ -503,11 +501,10 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, global_id, reply, reply + reply_len, session_key, session_key_len, con_secret, con_secret_len); - if (!ret) - set_global_id(ac, global_id); + WARN_ON(ret == -EAGAIN || ret > 0); mutex_unlock(&ac->mutex); return ret; } diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c index aab490a111eb..097e9f8d87a7 100644 --- a/net/ceph/auth_none.c +++ b/net/ceph/auth_none.c @@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end) * the generic auth code decode the global_id, and we carry no actual * authenticate state, so nothing happens here. */ -static int handle_reply(struct ceph_auth_client *ac, +static int handle_reply(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -77,6 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, struct ceph_auth_none_info *xi = ac->private; xi->starting = false; + ceph_auth_set_global_id(ac, global_id); return 0; } diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c index cab99c5581b0..b71b1635916e 100644 --- a/net/ceph/auth_x.c +++ b/net/ceph/auth_x.c @@ -597,7 +597,7 @@ bad: return -EINVAL; } -static int handle_auth_session_key(struct ceph_auth_client *ac, +static int handle_auth_session_key(struct ceph_auth_client *ac, u64 global_id, void **p, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -613,6 +613,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac, if (ret) return ret; + ceph_auth_set_global_id(ac, global_id); if (*p == end) { /* pre-nautilus (or didn't request service tickets!) */ WARN_ON(session_key || con_secret); @@ -661,7 +662,7 @@ e_inval: return -EINVAL; } -static int ceph_x_handle_reply(struct ceph_auth_client *ac, +static int ceph_x_handle_reply(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -695,9 +696,9 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, switch (op) { case CEPHX_GET_AUTH_SESSION_KEY: /* AUTH ticket + [connection secret] + service tickets */ - ret = handle_auth_session_key(ac, &p, end, session_key, - session_key_len, con_secret, - con_secret_len); + ret = handle_auth_session_key(ac, global_id, &p, end, + session_key, session_key_len, + con_secret, con_secret_len); break; case CEPHX_GET_PRINCIPAL_SESSION_KEY: -- cgit 1.4.1