From 8db2a44c9438749be98d41fb309f10d5084805df Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 17 Jan 2020 07:48:12 -0500 Subject: [PATCH] mount.ceph: remove arbitrary limit on size of name= option Aaron was getting back -ERANGE errors when trying to mount using a long name= option. The issue is that the destination buffer for the "key=" string is not big enough to hold long names. When I overhauled the mount.ceph code recently, I made this buffer much smaller than before figuring that it didn't need to be any larger than the length of "secret=". In the case where the secret is set in the keyring though, this buffer needs to be able to hold a string like "key=client.". The cephx name can be of arbitrary length, however. Rework the code to just safe_cat the strings directly into the options buffer, eliminating an extra copy and the need for an arbitrary limit. This also allows us to remove get_secret_option() from the the common code as well. Fixes: https://tracker.ceph.com/issues/43649 Reported-by: Aaron Signed-off-by: Jeff Layton --- src/common/secret.h | 7 ------ src/mount/mount.ceph.c | 45 ++++++++++++++++++++++++------------- src/mount/mount.ceph.h | 3 --- 4 files changed, 30 insertions(+), 75 deletions(-) diff --git a/src/common/secret.h b/src/common/secret.h index b681fa1ec7..5d2ad179dd 100644 --- a/src/common/secret.h +++ b/src/common/secret.h @@ -7,13 +7,6 @@ extern "C" { int read_secret_from_file(const char *filename, char *secret, size_t max_len); -/* - * Attempts to add the secret to the kernel, but falls back to - * the old secret= option if the kernel is too old. - */ -int get_secret_option(const char *secret, const char *key_name, - char *secret_option, size_t secret_option_len); - int set_kernel_secret(const char *secret, const char *key_name); int is_kernel_secret(const char *key_name); diff --git a/src/mount/mount.ceph.c b/src/mount/mount.ceph.c index e970648c59..133bc554f5 100644 --- a/src/mount/mount.ceph.c +++ b/src/mount/mount.ceph.c @@ -425,24 +425,39 @@ static void ceph_mount_info_free(struct ceph_mount_info *cmi) free(cmi->cmi_conf); } -static int finalize_options(struct ceph_mount_info *cmi) +static int append_key_or_secret_option(struct ceph_mount_info *cmi) { - int pos; + int pos = strlen(cmi->cmi_opts); - if (cmi->cmi_secret[0] || is_kernel_secret(cmi->cmi_name)) { - int ret; - char secret_option[SECRET_OPTION_BUFSIZE]; + if (!cmi->cmi_secret[0] && !is_kernel_secret(cmi->cmi_name)) + return 0; - ret = get_secret_option(cmi->cmi_secret, cmi->cmi_name, - secret_option, sizeof(secret_option)); - if (ret < 0) + if (pos) + pos = safe_cat(&cmi->cmi_opts, &cmi->cmi_opts_len, pos, ","); + + /* when parsing kernel options (-o remount) we get '' as the secret */ + if (cmi->cmi_secret[0] && (strcmp(cmi->cmi_secret, "") != 0)) { + int ret = set_kernel_secret(cmi->cmi_secret, cmi->cmi_name); + if (ret < 0) { + if (ret == -ENODEV || ret == -ENOSYS) { + /* old kernel; fall back to secret= in options */ + pos = safe_cat(&cmi->cmi_opts, + &cmi->cmi_opts_len, pos, + "secret="); + pos = safe_cat(&cmi->cmi_opts, + &cmi->cmi_opts_len, pos, + cmi->cmi_secret); + return 0; + } + fprintf(stderr, "adding ceph secret key to kernel failed: %s\n", + strerror(-ret)); return ret; - - pos = strlen(cmi->cmi_opts); - if (pos) - pos = safe_cat(&cmi->cmi_opts, &cmi->cmi_opts_len, pos, ","); - pos = safe_cat(&cmi->cmi_opts, &cmi->cmi_opts_len, pos, secret_option); + } } + + pos = safe_cat(&cmi->cmi_opts, &cmi->cmi_opts_len, pos, "key="); + pos = safe_cat(&cmi->cmi_opts, &cmi->cmi_opts_len, pos, cmi->cmi_name); + return 0; } @@ -493,9 +508,9 @@ int main(int argc, char *argv[]) /* Ensure the ceph key_type is available */ modprobe(); - retval = finalize_options(&cmi); + retval = append_key_or_secret_option(&cmi); if (retval) { - fprintf(stderr, "couldn't finalize options: %d\n", retval); + fprintf(stderr, "couldn't append secret option: %d\n", retval); retval = EX_USAGE; goto out; } diff --git a/src/mount/mount.ceph.h b/src/mount/mount.ceph.h index c563597c43..b61176923d 100644 --- a/src/mount/mount.ceph.h +++ b/src/mount/mount.ceph.h @@ -21,9 +21,6 @@ extern "C" { /* Max Including null terminator */ #define SECRET_BUFSIZE (MAX_SECRET_LEN + 1) -/* Buffer size for secret= option */ -#define SECRET_OPTION_BUFSIZE (sizeof("secret=") + MAX_SECRET_LEN + 1) - /* 2k should be enough for anyone? */ #define MON_LIST_BUFSIZE 2048 -- 2.24.1