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=<base64 encoded key>".

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.<cephx name>". 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 <aarongmldt@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/src/common/secret.c b/src/common/secret.c
index 9b821cb297ab87bad7137dee8868dceb4b76a014..fe34efdff83b25b6fcfe38231a9663529b3a2514 100644 (file)
--- a/src/common/secret.c
+++ b/src/common/secret.c
@@ -84,53 +84,3 @@ int is_kernel_secret(const char *key_name)
   serial = request_key("ceph", key_name, NULL, KEY_SPEC_USER_KEYRING);
   return serial != -1;
 }
-
-int get_secret_option(const char *secret, const char *key_name,
-                     char *secret_option, size_t max_len)
-{
-  if (!key_name) {
-    return -EINVAL;
-  }
-
-  int ret = 0;
-  int olen = strlen(key_name) + 7;
-  if (secret) {
-    olen += strlen(secret);
-  }
-  char option[olen+1];
-  int use_key = 1;
-
-  option[olen] = '\0';
-
-  /* when parsing kernel options (-o remount) we get '<hidden>' as the secret */
-  if (secret && (strcmp(secret, "<hidden>") != 0)) {
-    ret = set_kernel_secret(secret, key_name);
-    if (ret < 0) {
-      if (ret == -ENODEV || ret == -ENOSYS) {
-       /* running against older kernel; fall back to secret= in options */
-       snprintf(option, olen, "secret=%s", secret);
-       ret = 0;
-       use_key = 0;
-      } else {
-        char error_buf[80];
-       fprintf(stderr, "adding ceph secret key to kernel failed: %s.\n",
-               ceph_strerror_r(-ret, error_buf, sizeof(error_buf)));
-       return ret;
-      }
-    }
-  }
-
-  if (use_key) {
-    /* add key= option to identify key to use */
-    snprintf(option, olen, "key=%s", key_name);
-  }
-
-  if (strlen(option) + 1 > max_len) {
-    ret = -ERANGE;
-  } else {
-    secret_option[max_len-1] = '\0';
-    strncpy(secret_option, option, max_len-1);
-  }
-
-  return ret;
-}

diff --git a/src/common/secret.h b/src/common/secret.h
index b681fa1ec7ff2ed9e8e329b031cbde6cb1b7d205..5d2ad179dd0996af495f6cb0c5af5661f06dbc61 100644 (file)
--- 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 e970648c59d6db33e82a5f74665782a7e929aa04..133bc554f50882c46ffcd5b4215342b66e200d13 100644 (file)
--- 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 '<hidden>' as the secret */
+       if (cmi->cmi_secret[0] && (strcmp(cmi->cmi_secret, "<hidden>") != 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 c563597c43832d64a2cd16926fef161a4e982e03..b61176923d8dd5e1090244c90b0bb3158b6632d1 100644 (file)
--- 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