From: Marc Vef Date: Sun, 25 May 2025 19:02:50 +0000 (+0200) Subject: LU-19050 utils: Support long nid lists when getting fs info X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=fff7cd33bbb424b770768a71267604b4e87f7276;p=fs%2Flustre-release.git LU-19050 utils: Support long nid lists when getting fs info When "get_root_path_slow()" is called through various user commands, e.g., "lfs setquota", the internal "root_cache" is filled with mount point information. The cache's "nid" field allowed 256 characters which resulted in a buffer overflow for long nidlists that are set during mount. This patch removes this limitation and further removes the "nid" field from the "root_cache" since it is only needed in the "lfs check" command. Therefore, the nid list no longer needs to be processed and put into the cache in the numerous other llapi_* functions where the nid list is never accessed. Further, string copy handling was insufficient, allowing the overflow in the first place, and was updated accordingly for all fields. Signed-off-by: Marc Vef Change-Id: I3d9c30795fba14618368b7b9e1769fe0b07d3fc7 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/59421 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Feng Lei Reviewed-by: Oleg Drokin --- diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index 3c64214..fd4a911 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -82,8 +82,8 @@ #include #include #include -#include "lustreapi_internal.h" #include "lstddef.h" +#include "lustreapi_internal.h" #define FORMATTED_BUF_LEN 1024 @@ -7261,8 +7261,9 @@ static void do_target_check(char *obd_type_name, char *obd_name, int llapi_target_check(int type_num, char **obd_type, char *dir) { - char nid[MAX_LINE_LEN], instance[MAX_INSTANCE_LEN]; + char instance[MAX_INSTANCE_LEN]; struct check_target_filter filter = {NULL, NULL}; + char *nid = NULL; int rc; if (dir == NULL || dir[0] == '\0') @@ -7270,7 +7271,7 @@ int llapi_target_check(int type_num, char **obd_type, char *dir) do_target_check); rc = get_root_path(WANT_NID | WANT_ERROR, NULL, NULL, dir, -1, NULL, - nid); + &nid); if (rc) { llapi_error(LLAPI_MSG_ERROR, rc, "cannot get nid of path '%s'", dir); @@ -7280,11 +7281,16 @@ int llapi_target_check(int type_num, char **obd_type, char *dir) rc = llapi_get_instance(dir, instance, ARRAY_SIZE(instance)); if (rc) - return rc; + goto out; + filter.instance = instance; - return llapi_target_iterate(type_num, obd_type, &filter, + rc = llapi_target_iterate(type_num, obd_type, &filter, do_target_check); + +out: + free(nid); + return rc; } #undef MAX_STRING_SIZE diff --git a/lustre/utils/liblustreapi_root.c b/lustre/utils/liblustreapi_root.c index e413d49..a114b91 100644 --- a/lustre/utils/liblustreapi_root.c +++ b/lustre/utils/liblustreapi_root.c @@ -50,6 +50,7 @@ #include #include +#include #include #include #include "lustreapi_internal.h" @@ -59,7 +60,6 @@ static struct root_cache { dev_t dev; char fsname[PATH_MAX]; char mnt_dir[PATH_MAX]; - char nid[MAX_LINE_LEN]; int fd; /* cached fd on filesystem root for internal use only */ } root_cached = { 0 }; @@ -105,12 +105,30 @@ static int get_file_dev(const char *path, dev_t *dev) return 0; } +/** + * get_root_path_fast() - get mountpoint info from internal root_cache + * + * @want: bitmask of WANT_* flags indicating what information to return + * @fsname: if WANT_FSNAME is set, used as a buffer to return filesystem name + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @outfd: pointer to return open file descriptor (internal use only) + * @path: if WANT_PATH is set, used as a buffer to return mount point path + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @dev: pointer to return device ID + * + * Return: + * * %0 on success + * * %-ENODEV if no matching Lustre mount is found + * * %-ENAMETOOLONG if filesystem name, path, or NID is truncated + * * %negative error code on other failures + */ static int get_root_path_fast(int want, char *fsname, int *outfd, char *path, - dev_t *dev, char *nid) + dev_t *dev) { - int rc = -ENODEV; int fsnamelen; int mntlen; + int rc2; + int rc = -ENODEV; if (root_cached.dev == 0) return rc; @@ -145,10 +163,20 @@ static int get_root_path_fast(int want, char *fsname, int *outfd, char *path, if (rc) goto out_unlock; - if ((want & WANT_FSNAME) && fsname) - strcpy(fsname, root_cached.fsname); - if ((want & WANT_PATH) && path) - strcpy(path, root_cached.mnt_dir); + if ((want & WANT_FSNAME) && fsname) { + rc2 = scnprintf(fsname, PATH_MAX, "%s", root_cached.fsname); + if (rc2 < 0 || rc2 >= PATH_MAX - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + goto out_unlock; + } + } + if ((want & WANT_PATH) && path) { + rc2 = scnprintf(path, PATH_MAX, "%s", root_cached.mnt_dir); + if (rc2 < 0 || rc2 >= PATH_MAX - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + goto out_unlock; + } + } if ((want & WANT_DEV) && dev) *dev = root_cached.dev; if ((want & WANT_FD) && outfd) { @@ -160,36 +188,53 @@ static int get_root_path_fast(int want, char *fsname, int *outfd, char *path, root_cached.fd = *outfd; } } - if ((want & WANT_NID) && nid) - strcpy(nid, root_cached.nid); out_unlock: pthread_rwlock_unlock(&root_cached_lock); return rc; } +/** + * get_root_path_slow() - get mountpoint info from /proc/mounts and add to cache + * + * @want: bitmask of WANT_* flags indicating what information to return + * @fsname: if WANT_FSNAME is set, used as a buffer to return filesystem name + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @outfd: pointer to return open file descriptor (internal use only) + * @path: if WANT_PATH is set, used as a buffer to return mount point path + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @index: if WANT_INDEX is set, specifies which Lustre mount to find + * @dev: pointer to return device ID + * @out_nid: buffer to return a nidlist (dynamically allocated if rc == 0) + * + * Return: + * * %0 on success + * * %-ENODEV if no matching Lustre mount is found + * * %-ENAMETOOLONG if filesystem name, path, or NID is truncated + * * %negative error code on other failures + */ static int get_root_path_slow(int want, char *fsname, int *outfd, char *path, - int index, dev_t *dev, char *nid) + int index, dev_t *dev, char **out_nid) { struct mntent mnt; char buf[PATH_MAX]; char *ptr, *ptr_end; FILE *fp; int idx = -1, mntlen = 0; - int rc = -ENODEV; int fsnamelen = 0; dev_t devmnt = 0; + int rc2; + int rc = -ENODEV; /* get the mount point */ fp = setmntent(PROC_MOUNTS, "r"); - if (fp == NULL) { + if (!fp) { rc = -EIO; llapi_error(LLAPI_MSG_ERROR, rc, "cannot retrieve filesystem mount point"); return rc; } while (getmntent_r(fp, &mnt, buf, sizeof(buf))) { - if (!llapi_is_lustre_mnt(&mnt)) continue; @@ -203,7 +248,7 @@ static int get_root_path_slow(int want, char *fsname, int *outfd, char *path, * we are sure that mnt.mnt_fsname contains ":/", * so ptr should never be NULL */ - if (ptr == NULL) + if (!ptr) continue; ptr_end = ptr; while (*ptr_end != '/' && *ptr_end != '\0') @@ -267,27 +312,46 @@ static int get_root_path_slow(int want, char *fsname, int *outfd, char *path, } if ((want & WANT_FD) && outfd) rc = get_root_fd(mnt.mnt_dir, &root_cached.fd); - strncpy(root_cached.fsname, ptr, fsnamelen); - root_cached.fsname[fsnamelen] = '\0'; - strncpy(root_cached.mnt_dir, mnt.mnt_dir, mntlen); - root_cached.mnt_dir[mntlen] = '\0'; + + rc2 = scnprintf(root_cached.fsname, sizeof(root_cached.fsname), + "%.*s", fsnamelen, ptr); + if (rc2 < 0 || rc2 >= sizeof(root_cached.fsname) - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + goto unlock_root_cached; + } + + rc2 = scnprintf(root_cached.mnt_dir, + sizeof(root_cached.mnt_dir), "%.*s", mntlen, + mnt.mnt_dir); + if (rc2 < 0 || rc2 >= sizeof(root_cached.mnt_dir) - 1) + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + root_cached.dev = devmnt; - ptr_end = strstr(mnt.mnt_fsname, ":/"); - strncpy(root_cached.nid, mnt.mnt_fsname, - ptr_end - mnt.mnt_fsname); - root_cached.nid[ptr_end - mnt.mnt_fsname] = '\0'; + + /* if rc, cache was only partially updated and must be reset */ + if (rc) + memset(&root_cached, 0, sizeof(root_cached)); unlock_root_cached: pthread_rwlock_unlock(&root_cached_lock); + + if (rc) + goto out; } if ((want & WANT_FSNAME) && fsname) { - strncpy(fsname, ptr, fsnamelen); - fsname[fsnamelen] = '\0'; + rc2 = scnprintf(fsname, PATH_MAX, "%.*s", fsnamelen, ptr); + if (rc2 < 0 || rc2 >= PATH_MAX - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + goto out; + } } if ((want & WANT_PATH) && path) { - strncpy(path, mnt.mnt_dir, mntlen); - path[mntlen] = '\0'; + rc2 = scnprintf(path, PATH_MAX, "%.*s", mntlen, mnt.mnt_dir); + if (rc2 < 0 || rc2 >= PATH_MAX - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + goto out; + } } if ((want & WANT_DEV) && dev) *dev = devmnt; @@ -297,10 +361,27 @@ unlock_root_cached: else rc = get_root_fd(mnt.mnt_dir, outfd); } - if ((want & WANT_NID) && nid) { - ptr_end = strchr(mnt.mnt_fsname, ':'); - strncpy(nid, mnt.mnt_fsname, ptr_end - mnt.mnt_fsname); - nid[ptr_end - mnt.mnt_fsname] = '\0'; + + if (!rc && (want & WANT_NID) && out_nid) { + size_t nid_bufsz; + + ptr_end = strstr(mnt.mnt_fsname, ":/"); + nid_bufsz = ptr_end - mnt.mnt_fsname + 2; + *out_nid = malloc(nid_bufsz); + if (!*out_nid) { + rc = -ENOMEM; + goto out; + } + + rc2 = scnprintf(*out_nid, nid_bufsz, "%.*s", + (int)(ptr_end - mnt.mnt_fsname), + mnt.mnt_fsname); + if (rc2 < 0 || rc2 >= nid_bufsz - 1) { + rc = rc2 < 0 ? rc2 : -ENAMETOOLONG; + free(*out_nid); + *out_nid = NULL; + goto out; + } } out: @@ -308,28 +389,52 @@ out: return rc; } -/* +/** + * get_root_path() - find filesystem info using cached or slow lookup + * + * @want: bitmask of WANT_* flags indicating what information to return + * @fsname: if WANT_FSNAME is set, used as a buffer to return filesystem name + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @outfd: pointer to return open file descriptor (internal use only) + * @path: if WANT_PATH is set, used as a buffer to return mount point path + * (size is assumed to be PATH_MAX). Otherwise used to match the mount point + * @index: if WANT_INDEX is set, specifies which Lustre mount to find + * @dev: pointer to return device ID + * @out_nid: buffer to return a nidlist (dynamically allocated if rc == 0) + * * Find the fsname, the full path, and/or an open fd. * Either the fsname or path must not be NULL. * - * @outfd is for llapi internal use only, do not return it to the application. + * outfd is for llapi internal use only, do not return it to the application. + * + * Return: + * * %0 on success + * * %-ENODEV if no matching Lustre mount is found + * * %-ENAMETOOLONG if filesystem name, path, or NID is truncated + * * %negative error code on other failures */ int get_root_path(int want, char *fsname, int *outfd, char *path, int index, - dev_t *dev, char *nid) + dev_t *dev, char **out_nid) { int rc = -ENODEV; assert(fsname || path); - if (!(want & WANT_INDEX)) - rc = get_root_path_fast(want, fsname, outfd, path, dev, nid); - if (rc) + if (!(want & WANT_INDEX) || !(want & WANT_NID)) + rc = get_root_path_fast(want, fsname, outfd, path, dev); + if (rc || (want & WANT_NID)) rc = get_root_path_slow(want, fsname, outfd, path, index, dev, - nid); + out_nid); if (!rc || !(want & WANT_ERROR)) goto out_errno; + if (rc == -ENAMETOOLONG) { + llapi_err_noerrno(LLAPI_MSG_ERROR, + "filesystem name, path, or NID is too long"); + goto out_errno; + } + if (dev || !(want & WANT_DEV)) llapi_err_noerrno(LLAPI_MSG_ERROR, "'%u/%u' dev not on a mounted Lustre filesystem", @@ -342,6 +447,7 @@ out_errno: errno = -rc; return rc; } + /* * search lustre mounts * diff --git a/lustre/utils/lustreapi_internal.h b/lustre/utils/lustreapi_internal.h index a10d209..87eb8e0 100644 --- a/lustre/utils/lustreapi_internal.h +++ b/lustre/utils/lustreapi_internal.h @@ -47,7 +47,6 @@ #include #define MAX_IOC_BUFLEN 8192 -#define MAX_LINE_LEN 256 #define MAX_INSTANCE_LEN 32 #define WANT_PATH 0x1 @@ -69,7 +68,7 @@ #endif int get_root_path(int want, char *fsname, int *outfd, char *path, int index, - dev_t *dev, char *nid); + dev_t *dev, char **out_nid); int llapi_ioctl_pack(struct obd_ioctl_data *data, char **pbuf, int max_len); int llapi_ioctl_dev(int dev_id, unsigned int cmd, void *buf); int llapi_ioctl_unpack(struct obd_ioctl_data *data, char *pbuf, int max_len); diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index ba415ae..e57727c 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -6539,7 +6539,7 @@ int jt_pcc_del(int argc, char **argv) { .val = 'k', .name = "keep-data", .has_arg = no_argument }, { .val = 'v', .name = "verbose", .has_arg = no_argument }, { .name = NULL } }; - char fsname[MAX_OBD_NAME + 1]; + char fsname[PATH_MAX]; const char *mntpath; const char *pccpath; __u32 flags = PCC_CLEANUP_FL_NONE; @@ -6596,7 +6596,7 @@ int jt_pcc_clear(int argc, char **argv) { .val = 'k', .name = "keep-data", .has_arg = no_argument }, { .val = 'v', .name = "verbose", .has_arg = no_argument }, { .name = NULL } }; - char fsname[MAX_OBD_NAME + 1]; + char fsname[PATH_MAX]; const char *mntpath; __u32 flags = PCC_CLEANUP_FL_NONE; int verbose = LLAPI_MSG_INFO;