From d76eb66120abd100aceeef2682470d7ceae0845c Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 9 Oct 2026 14:14:58 -0400 Subject: [PATCH 1/5] ci: report coverage per component The plugins need vendor hardware and hold more than half of all lines. With one total, progress in the CLI, libnvme and the daemons is hard to see. Report a separate number for each part of the tree. Signed-off-by: Martin Belanger --- codecov.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/codecov.yml b/codecov.yml index 2ab0a1a649..d74d06ae6d 100644 --- a/codecov.yml +++ b/codecov.yml @@ -21,3 +21,34 @@ coverage: status: project: off patch: off + +# One coverage number per part of the tree. The plugins need vendor +# hardware and hold more than half of all lines. A separate number keeps +# the progress in the other parts visible. +component_management: + individual_components: + - component_id: cli + name: nvme CLI + paths: + - src/ + - component_id: libnvme + name: libnvme + paths: + - libnvme/src/ + - component_id: plugins + name: plugins + paths: + - plugins/ + - component_id: discoverd + name: nvme-discoverd + paths: + - discoverd/src/ + - component_id: keysd + name: nvme-keysd + paths: + - keysd/src/ + - component_id: common + name: common code + paths: + - shared/ + - daemon-util/ From 989a9f291885112dd826a5db799a9d2fc4a2923a Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 9 Oct 2026 15:58:21 -0400 Subject: [PATCH 2/5] tests: limit memory and run time of mock nvme runs Bad data from the mock device can make nvme allocate memory without bound or loop forever. One run grew to 26 GB and the kernel OOM killer stopped it and the terminal it ran in. Limit each run to 1 GB of memory and 60 seconds. With ASan, use hard_rss_limit_mb, because ASan reserves too much address space for RLIMIT_AS. Cross builds run nvme under qemu-user, which needs the whole guest address space, so they get no memory limit. Decode the output with replacement characters, because nvme prints the raw bytes of string fields. Signed-off-by: Martin Belanger --- tests/cli/nvme_mock_ipc.py | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/cli/nvme_mock_ipc.py b/tests/cli/nvme_mock_ipc.py index 4c6c5e81a3..534c4fb141 100644 --- a/tests/cli/nvme_mock_ipc.py +++ b/tests/cli/nvme_mock_ipc.py @@ -20,6 +20,7 @@ import errno import os import shlex +import shutil import socket import struct import subprocess @@ -215,6 +216,24 @@ def make_mock_env(mock_lib, ipc_sock_path): return env +# Limits for one nvme run. Bad data from the mock device can make nvme +# allocate without bound or loop forever. Without a limit, it can use all +# the memory of the machine. +_NVME_MEM_LIMIT_MB = 1024 +_NVME_TIMEOUT_S = 60 + + +def _is_asan_binary(nvme_bin): + """ASan reserves terabytes of address space, so RLIMIT_AS cannot be + used. Use ASan's own RSS limit instead.""" + path = shutil.which(nvme_bin) or nvme_bin + try: + with open(path, 'rb') as f: + return b'__asan_init' in f.read() + except OSError: + return False + + def run_nvme(nvme_bin, env, sysfs_dir, base_dir, *args, encoding='utf-8', stdin_data=None): """Runs `nvme_bin *args` under libmock_nvme.c. Returns the completed @@ -240,8 +259,20 @@ def run_nvme(nvme_bin, env, sysfs_dir, base_dir, *args, encoding='utf-8', # intact. env = dict(env) ld_preload = env.pop("LD_PRELOAD", "") + if os.environ.get('MESON_EXE_WRAPPER'): + # A cross build runs nvme under qemu-user, which needs the whole + # guest address space. + ulimit = '' + elif _is_asan_binary(nvme_bin): + env['ASAN_OPTIONS'] = ':'.join(filter(None, [ + env.get('ASAN_OPTIONS'), + f'hard_rss_limit_mb={_NVME_MEM_LIMIT_MB}'])) + ulimit = '' + else: + ulimit = f'ulimit -v {_NVME_MEM_LIMIT_MB * 1024}; ' wrapped_cmd = [ - '/bin/sh', '-c', f'export LD_PRELOAD={shlex.quote(ld_preload)}; exec "$@"', + '/bin/sh', '-c', + f'{ulimit}export LD_PRELOAD={shlex.quote(ld_preload)}; exec "$@"', 'nvme-mock-wrapper', ] + cmd @@ -251,7 +282,9 @@ def run_nvme(nvme_bin, env, sysfs_dir, base_dir, *args, encoding='utf-8', **stdin_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - encoding=encoding) + encoding=encoding, + errors='replace' if encoding else None, + timeout=_NVME_TIMEOUT_S) # Print outputs to sys.stderr so they are displayed by unittest on # failure. With encoding=None these are bytes, so summarise stdout From f0167cd51cd1bf075f6d5d6c25eeca3120b05028 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 9 Oct 2026 15:58:21 -0400 Subject: [PATCH 3/5] nvme-print: fix supported capacity configuration list log Only the 16-byte log header was read. The printers then read the descriptors from memory past the end of the buffer. They indexed the descriptors as fixed-size arrays, but each descriptor has a variable length. The counts in the log were not checked, so bad data made nvme read out of bounds and allocate gigabytes of memory. The log page has no length field. Read it again with a larger buffer until it holds all the descriptors, up to 256 KiB. Pass the length to the printers and stop at the end of the buffer. A Media Unit Descriptor Length that is odd leaves the next descriptors unaligned. The current specification requires this field to be 0, so copy each descriptor before reading it, in case a device or a future specification uses an odd value. Also fix the NVM Set index in the stdout output and the media unit keys in the JSON output (muid and mudl, not chanid and chmus). Signed-off-by: Martin Belanger --- plugins/log/log-plugin.c | 121 ++++++++++++++++++++-- src/nvme-print-binary.c | 4 +- src/nvme-print-json.c | 199 +++++++++++++++++++++++------------- src/nvme-print-stdout-log.c | 180 +++++++++++++++++++------------- src/nvme-print-stdout.h | 2 +- src/nvme-print.c | 6 +- src/nvme-print.h | 4 +- 7 files changed, 361 insertions(+), 155 deletions(-) diff --git a/plugins/log/log-plugin.c b/plugins/log/log-plugin.c index 539e10371d..ccb36f1839 100644 --- a/plugins/log/log-plugin.c +++ b/plugins/log/log-plugin.c @@ -1685,6 +1685,83 @@ static int get_media_unit_stat_log(int argc, char **argv, struct command *acmd, return err; } +#define SUPP_CAP_CONFIG_LOG_MAX_LEN (256 * 1024) + +/* + * The functions below return the offset after a descriptor that starts + * at @off. A value larger than @len means that the buffer is too short. + * An odd Media Unit Descriptor Length leaves the next descriptors + * unaligned, so they are copied before they are read. + */ +static size_t cap_config_chan_end(const unsigned char *buf, size_t off, + size_t len) +{ + struct nvme_channel_config_desc chd; + struct nvme_media_unit_config_desc mu; + + if (off + sizeof(chd) > len) + return off + sizeof(chd); + + memcpy(&chd, &buf[off], sizeof(chd)); + off += sizeof(chd); + + for (int m = 0; m < le16_to_cpu(chd.chmus); m++) { + if (off + sizeof(mu) > len) + return off + sizeof(mu); + + memcpy(&mu, &buf[off], sizeof(mu)); + off += sizeof(mu) + le16_to_cpu(mu.mudl); + } + + return off; +} + +static size_t cap_config_egcd_end(const unsigned char *buf, size_t off, + size_t len) +{ + struct nvme_end_grp_config_desc egcd; + struct nvme_end_grp_chan_desc chan_desc; + + if (off + sizeof(egcd) > len) + return off + sizeof(egcd); + + memcpy(&egcd, &buf[off], sizeof(egcd)); + off += sizeof(egcd) + le16_to_cpu(egcd.egsets) * sizeof(__le16); + + if (off + sizeof(chan_desc) > len) + return off + sizeof(chan_desc); + + memcpy(&chan_desc, &buf[off], sizeof(chan_desc)); + off += sizeof(chan_desc); + + for (int l = 0; l < le16_to_cpu(chan_desc.egchans) && off <= len; l++) + off = cap_config_chan_end(buf, off, len); + + return off; +} + +/* The log page has no length field. */ +static size_t supp_cap_config_log_len( + struct nvme_supported_cap_config_list_log *log, size_t len) +{ + const unsigned char *buf = (const unsigned char *)log; + struct nvme_capacity_config_desc desc; + size_t off = sizeof(*log); + + for (int i = 0; i < log->sccn && off <= len; i++) { + if (off + sizeof(desc) > len) + return off + sizeof(desc); + + memcpy(&desc, &buf[off], sizeof(desc)); + off += sizeof(desc); + + for (int j = 0; j < le16_to_cpu(desc.egcn) && off <= len; j++) + off = cap_config_egcd_end(buf, off, len); + } + + return off; +} + static int get_supp_cap_config_log(int argc, char **argv, struct command *acmd, struct plugin *plugin) { @@ -1693,6 +1770,7 @@ static int get_supp_cap_config_log(int argc, char **argv, struct command *acmd, __cleanup_libnvme_free struct nvme_supported_cap_config_list_log *cap_log = NULL; __cleanup_nvme_global_ctx struct libnvme_global_ctx *ctx = NULL; __cleanup_nvme_transport_handle struct libnvme_transport_handle *hdl = NULL; + size_t len = NVME_LOG_PAGE_PDU_SIZE, need; struct libnvme_passthru_cmd cmd; nvme_print_flags_t flags; int err = -1; @@ -1724,18 +1802,43 @@ static int get_supp_cap_config_log(int argc, char **argv, struct command *acmd, if (cfg.raw_binary) flags = BINARY; - cap_log = libnvme_alloc(sizeof(*cap_log)); - if (!cap_log) - return -ENOMEM; + /* + * Read the log again with a larger buffer until it holds all the + * descriptors. The data beyond the end of the log page is undefined + * and is not used. + */ + for (;;) { + void *p = libnvme_realloc(cap_log, len); - nvme_init_get_log_support_cap_config_list(&cmd, cfg.domainid, cap_log); - err = libnvme_get_log(hdl, &cmd, false, sizeof(*cap_log)); - if (err) { - nvme_show_err(err, "supported capacity configuration list log"); - return err; + if (!p) + return -ENOMEM; + cap_log = p; + + nvme_init_get_log_support_cap_config_list(&cmd, cfg.domainid, + cap_log); + cmd.data_len = len; + err = libnvme_get_log(hdl, &cmd, false, NVME_LOG_PAGE_PDU_SIZE); + if (err) { + nvme_show_err(err, + "supported capacity configuration list log"); + return err; + } + + need = supp_cap_config_log_len(cap_log, len); + if (need <= len) + break; + if (len == SUPP_CAP_CONFIG_LOG_MAX_LEN) { + nvme_show_error("log longer than %d bytes, truncated", + SUPP_CAP_CONFIG_LOG_MAX_LEN); + break; + } + need = (need + NVME_LOG_PAGE_PDU_SIZE - 1) & + ~(size_t)(NVME_LOG_PAGE_PDU_SIZE - 1); + len = min_t(size_t, max(need, 2 * len), + SUPP_CAP_CONFIG_LOG_MAX_LEN); } - nvme_show_supported_cap_config_log(cap_log, flags); + nvme_show_supported_cap_config_log(cap_log, len, flags); return err; } diff --git a/src/nvme-print-binary.c b/src/nvme-print-binary.c index e075471b4d..847a6b7d23 100644 --- a/src/nvme-print-binary.c +++ b/src/nvme-print-binary.c @@ -104,9 +104,9 @@ static void binary_fdp_ruh_status(struct nvme_fdp_ruh_status *status, size_t len } static void binary_supported_cap_config_log( - struct nvme_supported_cap_config_list_log *cap) + struct nvme_supported_cap_config_list_log *cap, size_t len) { - d_raw((unsigned char *)cap, sizeof(*cap)); + d_raw((unsigned char *)cap, len); } static void binary_ctrl_registers(void *bar, bool fabrics) diff --git a/src/nvme-print-json.c b/src/nvme-print-json.c index 29089a24b9..448a0dc1d6 100644 --- a/src/nvme-print-json.c +++ b/src/nvme-print-json.c @@ -2527,87 +2527,148 @@ static void json_media_unit_stat_log(struct nvme_media_unit_stat_log *mus) obj_add_array(r, "mus_list", entries); } +/* + * An odd Media Unit Descriptor Length leaves the next descriptors + * unaligned, so they are copied before they are read. + */ +static bool json_cap_config_chan(struct json_object *chan_list, + unsigned char **p, unsigned char *end) +{ + struct nvme_channel_config_desc chd; + struct json_object *channel, *media_list; + int chmus; + + if (!shr_buf_has_room(*p, end, sizeof(chd))) + return false; + + memcpy(&chd, *p, sizeof(chd)); + chmus = le16_to_cpu(chd.chmus); + *p += sizeof(chd); + + channel = json_create_object(); + obj_add_uint(channel, "chanid", le16_to_cpu(chd.chanid)); + obj_add_uint(channel, "chmus", chmus); + media_list = json_create_array(); + obj_add_array(channel, "Media Descriptor", media_list); + array_add_obj(chan_list, channel); + + for (int m = 0; m < chmus; m++) { + struct nvme_media_unit_config_desc mu; + struct json_object *media; + size_t size; + + if (!shr_buf_has_room(*p, end, sizeof(mu))) + return false; + + memcpy(&mu, *p, sizeof(mu)); + size = sizeof(mu) + le16_to_cpu(mu.mudl); + if (!shr_buf_has_room(*p, end, size)) + return false; + + media = json_create_object(); + obj_add_uint(media, "muid", le16_to_cpu(mu.muid)); + obj_add_uint(media, "mudl", le16_to_cpu(mu.mudl)); + array_add_obj(media_list, media); + *p += size; + } + + return true; +} + +static bool json_cap_config_egcd(struct json_object *end_list, + unsigned char **p, unsigned char *end) +{ + struct nvme_end_grp_config_desc egcd; + struct nvme_end_grp_chan_desc chan_desc; + struct json_object *endurance, *set_list, *chan_list; + unsigned char *sets; + int egsets, egchans; + + if (!shr_buf_has_room(*p, end, sizeof(egcd))) + return false; + + memcpy(&egcd, *p, sizeof(egcd)); + egsets = le16_to_cpu(egcd.egsets); + sets = *p + sizeof(egcd); + *p = sets + egsets * sizeof(__le16); + if (!shr_buf_has_room(sets, end, egsets * sizeof(__le16) + + sizeof(chan_desc))) + return false; + + memcpy(&chan_desc, *p, sizeof(chan_desc)); + egchans = le16_to_cpu(chan_desc.egchans); + *p += sizeof(chan_desc); + + endurance = json_create_object(); + obj_add_uint(endurance, "endgid", le16_to_cpu(egcd.endgid)); + obj_add_uint(endurance, "cap_adj_factor", + le16_to_cpu(egcd.cap_adj_factor)); + obj_add_uint128(endurance, "tegcap", le128_to_cpu(egcd.tegcap)); + obj_add_uint128(endurance, "segcap", le128_to_cpu(egcd.segcap)); + obj_add_uint(endurance, "egsets", egsets); + obj_add_uint(endurance, "egchans", egchans); + + set_list = json_create_array(); + for (int k = 0; k < egsets; k++) { + struct json_object *set = json_create_object(); + __le16 id; + + memcpy(&id, sets + k * sizeof(id), sizeof(id)); + obj_add_uint(set, "nvmsetid", le16_to_cpu(id)); + array_add_obj(set_list, set); + } + + chan_list = json_create_array(); + obj_add_array(endurance, "Channel Descriptor", chan_list); + obj_add_array(endurance, "NVM Set IDs", set_list); + array_add_obj(end_list, endurance); + + for (int l = 0; l < egchans; l++) { + if (!json_cap_config_chan(chan_list, p, end)) + return false; + } + + return true; +} + static void json_supported_cap_config_log( - struct nvme_supported_cap_config_list_log *cap_log) + struct nvme_supported_cap_config_list_log *cap_log, size_t len) { struct json_object *r = json_r; struct json_object *cap_list = json_create_array(); - struct json_object *capacity; - struct json_object *end_list; - struct json_object *set_list; - struct json_object *set; - struct json_object *chan_list; - struct json_object *channel; - struct json_object *media_list; - struct json_object *media; - struct json_object *endurance; - struct nvme_end_grp_chan_desc *chan_desc; - int i, j, k, l, m, egcn, egsets, egchans, chmus; - int sccn = cap_log->sccn; + unsigned char *p = (unsigned char *)cap_log->cap_config_desc; + unsigned char *end = (unsigned char *)cap_log + len; + bool more = true; + + obj_add_array(r, "Capacity Descriptor", cap_list); + if (len < sizeof(*cap_log)) + return; obj_add_uint(r, "sccn", cap_log->sccn); - for (i = 0; i < sccn; i++) { + for (int i = 0; more && i < cap_log->sccn; i++) { + struct nvme_capacity_config_desc desc; + struct json_object *capacity, *end_list; + int egcn; + + if (!shr_buf_has_room(p, end, sizeof(desc))) + break; + + memcpy(&desc, p, sizeof(desc)); + egcn = le16_to_cpu(desc.egcn); + p += sizeof(desc); + capacity = json_create_object(); obj_add_uint(capacity, "cap_config_id", - le16_to_cpu(cap_log->cap_config_desc[i].cap_config_id)); - obj_add_uint(capacity, "domainid", - le16_to_cpu(cap_log->cap_config_desc[i].domainid)); - obj_add_uint(capacity, "egcn", le16_to_cpu(cap_log->cap_config_desc[i].egcn)); + le16_to_cpu(desc.cap_config_id)); + obj_add_uint(capacity, "domainid", le16_to_cpu(desc.domainid)); + obj_add_uint(capacity, "egcn", egcn); end_list = json_create_array(); - egcn = le16_to_cpu(cap_log->cap_config_desc[i].egcn); - for (j = 0; j < egcn; j++) { - endurance = json_create_object(); - obj_add_uint(endurance, "endgid", - le16_to_cpu(cap_log->cap_config_desc[i].egcd[j].endgid)); - obj_add_uint(endurance, "cap_adj_factor", - le16_to_cpu(cap_log->cap_config_desc[i].egcd[j].cap_adj_factor)); - obj_add_uint128(endurance, "tegcap", - le128_to_cpu(cap_log->cap_config_desc[i].egcd[j].tegcap)); - obj_add_uint128(endurance, "segcap", - le128_to_cpu(cap_log->cap_config_desc[i].egcd[j].segcap)); - obj_add_uint(endurance, "egsets", - le16_to_cpu(cap_log->cap_config_desc[i].egcd[j].egsets)); - egsets = le16_to_cpu(cap_log->cap_config_desc[i].egcd[j].egsets); - set_list = json_create_array(); - for (k = 0; k < egsets; k++) { - set = json_create_object(); - obj_add_uint(set, "nvmsetid", - le16_to_cpu(cap_log->cap_config_desc[i].egcd[j].nvmsetid[k])); - array_add_obj(set_list, set); - } - chan_desc = (struct nvme_end_grp_chan_desc *) - &cap_log->cap_config_desc[i].egcd[j].nvmsetid[egsets]; - egchans = le16_to_cpu(chan_desc->egchans); - obj_add_uint(endurance, "egchans", le16_to_cpu(chan_desc->egchans)); - chan_list = json_create_array(); - for (l = 0; l < egchans; l++) { - channel = json_create_object(); - obj_add_uint(channel, "chanid", - le16_to_cpu(chan_desc->chan_config_desc[l].chanid)); - obj_add_uint(channel, "chmus", - le16_to_cpu(chan_desc->chan_config_desc[l].chmus)); - chmus = le16_to_cpu(chan_desc->chan_config_desc[l].chmus); - media_list = json_create_array(); - for (m = 0; m < chmus; m++) { - media = json_create_object(); - obj_add_uint(media, "chanid", - le16_to_cpu(chan_desc->chan_config_desc[l].mu_config_desc[m].muid)); - obj_add_uint(media, "chmus", - le16_to_cpu(chan_desc->chan_config_desc[l].mu_config_desc[m].mudl)); - array_add_obj(media_list, media); - } - obj_add_array(channel, "Media Descriptor", media_list); - array_add_obj(chan_list, channel); - } - obj_add_array(endurance, "Channel Descriptor", chan_list); - obj_add_array(endurance, "NVM Set IDs", set_list); - array_add_obj(end_list, endurance); - } obj_add_array(capacity, "Endurance Descriptor", end_list); array_add_obj(cap_list, capacity); - } - obj_add_array(r, "Capacity Descriptor", cap_list); + for (int j = 0; more && j < egcn; j++) + more = json_cap_config_egcd(end_list, &p, end); + } } static void json_nvme_fdp_configs(struct nvme_fdp_config_log *log, size_t len) diff --git a/src/nvme-print-stdout-log.c b/src/nvme-print-stdout-log.c index e9994d2930..be6ffb6638 100644 --- a/src/nvme-print-stdout-log.c +++ b/src/nvme-print-stdout-log.c @@ -1520,94 +1520,133 @@ void stdout_fdp_events(struct nvme_fdp_events_log *log) } } -static void stdout_supported_cap_config_add_channels(struct shr_table *t, - struct nvme_end_grp_chan_desc *chan_desc) +/* + * An odd Media Unit Descriptor Length leaves the next descriptors + * unaligned, so they are copied before they are read. + */ +static bool stdout_supported_cap_config_add_channel(struct shr_table *t, + unsigned char **p, unsigned char *end) { - int egchans = le16_to_cpu(chan_desc->egchans); + struct nvme_channel_config_desc chd; + int chmus; - stdout_kv_add(t, "Number of Channels", "%u", egchans); + if (!shr_buf_has_room(*p, end, sizeof(chd))) + return false; - for (int l = 0; l < egchans; l++) { - struct nvme_channel_config_desc *chd = - &chan_desc->chan_config_desc[l]; - int chmus = le16_to_cpu(chd->chmus); - - stdout_kv_add(t, "Channel Identifier", "%u", - le16_to_cpu(chd->chanid)); - stdout_kv_add(t, "Number of Channel Media Units", "%u", chmus); - - for (int m = 0; m < chmus; m++) { - struct nvme_media_unit_config_desc *mu = - &chd->mu_config_desc[m]; - - stdout_kv_add(t, "Media Unit Identifier", "%u", - le16_to_cpu(mu->muid)); - stdout_kv_add(t, "Media Unit Descriptor Length", "%u", - le16_to_cpu(mu->mudl)); - } - } -} + memcpy(&chd, *p, sizeof(chd)); + chmus = le16_to_cpu(chd.chmus); + *p += sizeof(chd); -static void stdout_supported_cap_config_add_egcd(struct shr_table *t, - struct nvme_supported_cap_config_list_log *cap, int i) -{ - struct nvme_end_grp_chan_desc *chan_desc; - int egcn = le16_to_cpu(cap->cap_config_desc[i].egcn); + stdout_kv_add(t, "Channel Identifier", "%u", le16_to_cpu(chd.chanid)); + stdout_kv_add(t, "Number of Channel Media Units", "%u", chmus); - stdout_kv_add(t, "Number of Endurance Group Configuration Descriptors", - "%u", egcn); + for (int m = 0; m < chmus; m++) { + struct nvme_media_unit_config_desc mu; + size_t size; - for (int j = 0; j < egcn; j++) { - struct nvme_end_grp_config_desc *egcd = - &cap->cap_config_desc[i].egcd[j]; - int egsets = le16_to_cpu(egcd->egsets); + if (!shr_buf_has_room(*p, end, sizeof(mu))) + return false; - stdout_kv_add(t, "Endurance Group Identifier", "%u", - le16_to_cpu(egcd->endgid)); - stdout_kv_add(t, "Capacity Adjustment Factor", "%u", - le16_to_cpu(egcd->cap_adj_factor)); - stdout_kv_add(t, "Total Endurance Group Capacity", "%s", - uint128_t_to_l10n_string( - le128_to_cpu(egcd->tegcap))); - stdout_kv_add(t, "Spare Endurance Group Capacity", "%s", - uint128_t_to_l10n_string( - le128_to_cpu(egcd->segcap))); - stdout_kv_add(t, "Endurance Estimate", "%s", - uint128_t_to_l10n_string( - le128_to_cpu(egcd->end_est))); - stdout_kv_add(t, "Number of NVM Sets", "%u", egsets); - - for (int k = 0; k < egsets; k++) { - char name[32]; - - snprintf(name, sizeof(name), - "NVM Set %d Identifier", i); - stdout_kv_add(t, name, "%u", - le16_to_cpu(egcd->nvmsetid[k])); - } + memcpy(&mu, *p, sizeof(mu)); + size = sizeof(mu) + le16_to_cpu(mu.mudl); + if (!shr_buf_has_room(*p, end, size)) + return false; + + stdout_kv_add(t, "Media Unit Identifier", "%u", + le16_to_cpu(mu.muid)); + stdout_kv_add(t, "Media Unit Descriptor Length", "%u", + le16_to_cpu(mu.mudl)); + *p += size; + } - chan_desc = (struct nvme_end_grp_chan_desc *) - &egcd->nvmsetid[egsets]; - stdout_supported_cap_config_add_channels(t, chan_desc); + return true; +} + +static bool stdout_supported_cap_config_add_egcd(struct shr_table *t, + unsigned char **p, unsigned char *end) +{ + struct nvme_end_grp_config_desc egcd; + struct nvme_end_grp_chan_desc chan_desc; + unsigned char *sets; + int egsets, egchans; + + if (!shr_buf_has_room(*p, end, sizeof(egcd))) + return false; + + memcpy(&egcd, *p, sizeof(egcd)); + egsets = le16_to_cpu(egcd.egsets); + sets = *p + sizeof(egcd); + *p = sets + egsets * sizeof(__le16); + if (!shr_buf_has_room(sets, end, egsets * sizeof(__le16) + + sizeof(chan_desc))) + return false; + + memcpy(&chan_desc, *p, sizeof(chan_desc)); + egchans = le16_to_cpu(chan_desc.egchans); + *p += sizeof(chan_desc); + + stdout_kv_add(t, "Endurance Group Identifier", "%u", + le16_to_cpu(egcd.endgid)); + stdout_kv_add(t, "Capacity Adjustment Factor", "%u", + le16_to_cpu(egcd.cap_adj_factor)); + stdout_kv_add(t, "Total Endurance Group Capacity", "%s", + uint128_t_to_l10n_string(le128_to_cpu(egcd.tegcap))); + stdout_kv_add(t, "Spare Endurance Group Capacity", "%s", + uint128_t_to_l10n_string(le128_to_cpu(egcd.segcap))); + stdout_kv_add(t, "Endurance Estimate", "%s", + uint128_t_to_l10n_string(le128_to_cpu(egcd.end_est))); + stdout_kv_add(t, "Number of NVM Sets", "%u", egsets); + + for (int k = 0; k < egsets; k++) { + char name[32]; + __le16 id; + + memcpy(&id, sets + k * sizeof(id), sizeof(id)); + snprintf(name, sizeof(name), "NVM Set %d Identifier", k); + stdout_kv_add(t, name, "%u", le16_to_cpu(id)); } + + stdout_kv_add(t, "Number of Channels", "%u", egchans); + + for (int l = 0; l < egchans; l++) { + if (!stdout_supported_cap_config_add_channel(t, p, end)) + return false; + } + + return true; } void stdout_supported_cap_config_log( - struct nvme_supported_cap_config_list_log *cap) + struct nvme_supported_cap_config_list_log *cap, size_t len) { - int sccn = cap->sccn; + unsigned char *p = (unsigned char *)cap->cap_config_desc; + unsigned char *end = (unsigned char *)cap + len; struct shr_table *t; + bool more = true; + + if (len < sizeof(*cap)) + return; t = stdout_kv_table_create(); if (!t) return; stdout_kv_add(t, "Number of Supported Capacity Configurations", "%u", - sccn); + cap->sccn); stdout_kv_table_finish(t, "supported-cap-config"); - for (int i = 0; i < sccn; i++) { + for (int i = 0; more && i < cap->sccn; i++) { + struct nvme_capacity_config_desc desc; + int egcn; + + if (!shr_buf_has_room(p, end, sizeof(desc))) + break; + + memcpy(&desc, p, sizeof(desc)); + egcn = le16_to_cpu(desc.egcn); + p += sizeof(desc); + printf("Capacity Configuration Descriptor: %u\n", i); t = stdout_kv_table_create(); @@ -1615,12 +1654,15 @@ void stdout_supported_cap_config_log( return; stdout_kv_add(t, "Capacity Configuration Identifier", "%u", - le16_to_cpu( - cap->cap_config_desc[i].cap_config_id)); + le16_to_cpu(desc.cap_config_id)); stdout_kv_add(t, "Domain Identifier", "%u", - le16_to_cpu(cap->cap_config_desc[i].domainid)); + le16_to_cpu(desc.domainid)); + stdout_kv_add(t, + "Number of Endurance Group Configuration Descriptors", + "%u", egcn); - stdout_supported_cap_config_add_egcd(t, cap, i); + for (int j = 0; more && j < egcn; j++) + more = stdout_supported_cap_config_add_egcd(t, &p, end); stdout_kv_table_finish(t, "supported-cap-config"); } diff --git a/src/nvme-print-stdout.h b/src/nvme-print-stdout.h index 9c62091388..9ede1fe921 100644 --- a/src/nvme-print-stdout.h +++ b/src/nvme-print-stdout.h @@ -83,7 +83,7 @@ void stdout_self_test_log(struct nvme_self_test_log *self_test, void stdout_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char *devname); void stdout_supported_cap_config_log( - struct nvme_supported_cap_config_list_log *cap); + struct nvme_supported_cap_config_list_log *cap, size_t len); void stdout_supported_log(struct nvme_supported_log_pages *support_log, const char *devname); void stdout_zns_changed(struct nvme_zns_changed_zone_log *log); diff --git a/src/nvme-print.c b/src/nvme-print.c index 45013e93ad..e011f6a3d6 100644 --- a/src/nvme-print.c +++ b/src/nvme-print.c @@ -446,10 +446,10 @@ void nvme_show_fdp_ruh_status(struct nvme_fdp_ruh_status *status, size_t len, } void nvme_show_supported_cap_config_log( - struct nvme_supported_cap_config_list_log *cap, + struct nvme_supported_cap_config_list_log *cap, size_t len, nvme_print_flags_t flags) { - nvme_print(supported_cap_config_list_log, flags, cap); + nvme_print(supported_cap_config_list_log, flags, cap, len); } void nvme_show_subsystem_list(struct libnvme_global_ctx *ctx, bool show_ana, @@ -2040,7 +2040,7 @@ void nvme_show_log(const char *devname, enum nvme_cmd_get_log_lid lid, __u32 nsi nvme_show_media_unit_stat_log(log, flags); break; case NVME_LOG_LID_SUPPORTED_CAP_CONFIG_LIST: - nvme_show_supported_cap_config_log(log, flags); + nvme_show_supported_cap_config_log(log, len, flags); break; case NVME_LOG_LID_FID_SUPPORTED_EFFECTS: nvme_show_fid_support_effects_log(log, devname, flags); diff --git a/src/nvme-print.h b/src/nvme-print.h index 5a976a9afc..31fe237d37 100644 --- a/src/nvme-print.h +++ b/src/nvme-print.h @@ -176,7 +176,7 @@ struct print_ops { void (*self_test_log)(struct nvme_self_test_log *self_test, __u8 dst_entries, __u32 size, const char *devname); void (*single_property)(int offset, uint64_t value64); void (*smart_log)(struct nvme_smart_log *smart, unsigned int nsid, const char *devname); - void (*supported_cap_config_list_log)(struct nvme_supported_cap_config_list_log *cap_log); + void (*supported_cap_config_list_log)(struct nvme_supported_cap_config_list_log *cap_log, size_t len); void (*supported_log_pages)(struct nvme_supported_log_pages *support_log, const char *devname); void (*zns_start_zone_list)(__u64 nr_zones, struct json_object **zone_list); void (*zns_changed_zone_log)(struct nvme_zns_changed_zone_log *log); @@ -395,7 +395,7 @@ void nvme_show_mi_cmd_support_effects_log(struct nvme_mi_cmd_supported_effects_l void nvme_show_media_unit_stat_log(struct nvme_media_unit_stat_log *mus, nvme_print_flags_t flags); void nvme_show_supported_cap_config_log(struct nvme_supported_cap_config_list_log *caplog, - nvme_print_flags_t flags); + size_t len, nvme_print_flags_t flags); void nvme_show_ctrl_registers(void *bar, bool fabrics, nvme_print_flags_t flags); void nvme_show_ctrl_register(void *bar, bool fabrics, int offset, nvme_print_flags_t flags); void nvme_show_single_property(int offset, uint64_t prop, nvme_print_flags_t flags); From 6b398f2caf716b6950805c8bf5505c642da20044 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 9 Oct 2026 16:32:02 -0400 Subject: [PATCH 4/5] nvme-print-json: do not crash on missing sysfs attributes The JSON printers read sysfs attributes with a NULL default and pass the result to obj_add_str(), which uses it as a format string. When the attribute does not exist, nvme crashes. This happens on real systems: ana_state is missing for PCIe multipath without ANA, iopolicy without native multipath, and phy_slot for controllers that are not PCIe. "nvme list -v -o json" and "nvme show-topology -o json" crash on such systems. Use an empty string as the default, as the stdout printers do. Signed-off-by: Martin Belanger --- src/nvme-print-json.c | 56 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/nvme-print-json.c b/src/nvme-print-json.c index 448a0dc1d6..5fb598e809 100644 --- a/src/nvme-print-json.c +++ b/src/nvme-print-json.c @@ -2854,7 +2854,7 @@ static unsigned int json_print_nvme_subsystem_multipath(struct libnvme_subsystem struct libnvme_ctrl *c = libnvme_path_get_ctrl(p); const char *ana_state; - libnvme_path_get_ana_state(p, &ana_state, NULL); + libnvme_path_get_ana_state(p, &ana_state, ""); path_attrs = json_create_object(); obj_add_str(path_attrs, "Name", libnvme_ctrl_get_name(c)); obj_add_str(path_attrs, "Transport", libnvme_ctrl_get_transport(c)); @@ -2924,12 +2924,12 @@ static void json_print_nvme_subsystem_list(struct libnvme_global_ctx *ctx, const char *firmware; const char *iopolicy; - libnvme_subsystem_get_model(s, &model, NULL); - libnvme_subsystem_get_serial(s, &serial, NULL); + libnvme_subsystem_get_model(s, &model, ""); + libnvme_subsystem_get_serial(s, &serial, ""); libnvme_subsystem_get_firmware(s, &firmware, - NULL); + ""); libnvme_subsystem_get_iopolicy(s, &iopolicy, - NULL); + ""); obj_add_str(subsystem_attrs, "Model", model); obj_add_str(subsystem_attrs, "Serial", serial); @@ -4868,7 +4868,7 @@ static void json_print_detail_list_multipath(struct libnvme_subsystem *s, const char *firmware; const char *ana_state; - libnvme_path_get_ana_state(p, &ana_state, NULL); + libnvme_path_get_ana_state(p, &ana_state, ""); obj_add_str(jpath, "Path", libnvme_path_get_name(p)); obj_add_str(jpath, "ANAState", ana_state); @@ -4878,11 +4878,11 @@ static void json_print_detail_list_multipath(struct libnvme_subsystem *s, * controller attributes. */ c = libnvme_path_get_ctrl(p); - libnvme_ctrl_get_phy_slot(c, &slot, NULL); - libnvme_ctrl_get_cntlid(c, &cntlid, NULL); - libnvme_ctrl_get_serial(c, &serial, NULL); - libnvme_ctrl_get_model(c, &model, NULL); - libnvme_ctrl_get_firmware(c, &firmware, NULL); + libnvme_ctrl_get_phy_slot(c, &slot, ""); + libnvme_ctrl_get_cntlid(c, &cntlid, ""); + libnvme_ctrl_get_serial(c, &serial, ""); + libnvme_ctrl_get_model(c, &model, ""); + libnvme_ctrl_get_firmware(c, &firmware, ""); obj_add_str(jpath, "Controller", libnvme_ctrl_get_name(c)); obj_add_str(jpath, "Cntlid", cntlid); obj_add_str(jpath, "SerialNumber", serial); @@ -4917,11 +4917,11 @@ static void json_print_detail_list(struct libnvme_subsystem *s, struct json_obje const char *model; const char *firmware; - libnvme_ctrl_get_phy_slot(c, &slot, NULL); - libnvme_ctrl_get_cntlid(c, &cntlid, NULL); - libnvme_ctrl_get_serial(c, &serial, NULL); - libnvme_ctrl_get_model(c, &model, NULL); - libnvme_ctrl_get_firmware(c, &firmware, NULL); + libnvme_ctrl_get_phy_slot(c, &slot, ""); + libnvme_ctrl_get_cntlid(c, &cntlid, ""); + libnvme_ctrl_get_serial(c, &serial, ""); + libnvme_ctrl_get_model(c, &model, ""); + libnvme_ctrl_get_firmware(c, &firmware, ""); obj_add_str(jctrl, "Controller", libnvme_ctrl_get_name(c)); obj_add_str(jctrl, "Cntlid", cntlid); @@ -5040,11 +5040,11 @@ static void json_detail_list(struct libnvme_global_ctx *ctx) const char *firmware; const char *slot; - libnvme_ctrl_get_cntlid(c, &cntlid, NULL); - libnvme_ctrl_get_serial(c, &serial, NULL); - libnvme_ctrl_get_model(c, &model, NULL); - libnvme_ctrl_get_firmware(c, &firmware, NULL); - libnvme_ctrl_get_phy_slot(c, &slot, NULL); + libnvme_ctrl_get_cntlid(c, &cntlid, ""); + libnvme_ctrl_get_serial(c, &serial, ""); + libnvme_ctrl_get_model(c, &model, ""); + libnvme_ctrl_get_firmware(c, &firmware, ""); + libnvme_ctrl_get_phy_slot(c, &slot, ""); obj_add_str(jctrl, "Controller", libnvme_ctrl_get_name(c)); obj_add_str(jctrl, "Cntlid", cntlid); @@ -5086,7 +5086,7 @@ static void json_detail_list(struct libnvme_global_ctx *ctx) const char *ana_state; libnvme_path_get_ana_state(p, - &ana_state, NULL); + &ana_state, ""); obj_add_str(jpath, "Path", libnvme_path_get_name(p)); obj_add_str(jpath, "ANAState", ana_state); @@ -5231,7 +5231,7 @@ static unsigned int json_subsystem_topology_multipath(struct libnvme_subsystem * struct libnvme_ctrl *c; const char *ana_state; - libnvme_path_get_ana_state(p, &ana_state, NULL); + libnvme_path_get_ana_state(p, &ana_state, ""); path_attrs = json_create_object(); obj_add_str(path_attrs, "Path", libnvme_path_get_name(p)); obj_add_str(path_attrs, "ANAState", ana_state); @@ -5245,7 +5245,7 @@ static unsigned int json_subsystem_topology_multipath(struct libnvme_subsystem * const char *numa_nodes; libnvme_path_get_numa_nodes(p, &numa_nodes, - NULL); + ""); obj_add_str(path_attrs, "NUMANodes", numa_nodes); } else if (!strcmp(iopolicy, "queue-depth")) { @@ -5335,7 +5335,7 @@ static void json_simple_topology(struct libnvme_global_ctx *ctx) subsystem_attrs = json_create_object(); obj_add_str(subsystem_attrs, "Name", libnvme_subsystem_get_name(s)); obj_add_str(subsystem_attrs, "NQN", libnvme_subsystem_get_subsysnqn(s)); - libnvme_subsystem_get_iopolicy(s, &iopolicy, NULL); + libnvme_subsystem_get_iopolicy(s, &iopolicy, ""); obj_add_str(subsystem_attrs, "IOPolicy", iopolicy); if (verbose_mode()) { @@ -5343,10 +5343,10 @@ static void json_simple_topology(struct libnvme_global_ctx *ctx) const char *serial; const char *firmware; - libnvme_subsystem_get_model(s, &model, NULL); - libnvme_subsystem_get_serial(s, &serial, NULL); + libnvme_subsystem_get_model(s, &model, ""); + libnvme_subsystem_get_serial(s, &serial, ""); libnvme_subsystem_get_firmware(s, &firmware, - NULL); + ""); obj_add_str(subsystem_attrs, "Model", model); obj_add_str(subsystem_attrs, "Serial", serial); From ae32ce1e14863589abb21600afdbff17e8a274fd Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 9 Oct 2026 15:58:21 -0400 Subject: [PATCH 5/5] tests: run the read-only commands in every output format Most of the print code is not covered by tests. Run the id, log, feat and get-feature commands, and the read-only fdp, zns, resv and dir commands, on the mock device in the normal, verbose, JSON and binary formats. The mock device returns pseudo-random data from a fixed seed, so the print code takes many branches. Each command must exit without a crash, and the JSON output must parse. Also check the supported capacity configuration list log and the persistent event log with valid data, and run the list and topology commands on sysfs trees captured from real systems. The test does not run on big-endian hosts yet. It times out on s390x. Signed-off-by: Martin Belanger --- tests/cli/meson.build | 14 ++ tests/cli/nvme_print_test.py | 336 +++++++++++++++++++++++++++++++++++ 2 files changed, 350 insertions(+) create mode 100644 tests/cli/nvme_print_test.py diff --git a/tests/cli/meson.build b/tests/cli/meson.build index 9dcf97000b..3e6167bf58 100644 --- a/tests/cli/meson.build +++ b/tests/cli/meson.build @@ -106,6 +106,20 @@ if python3.found() env: cli_test_env, depends: [nvme_exe, mock_nvme_lib]) + # Not on big-endian hosts yet: the test times out on s390x. + if 'id' in selected_plugins and 'log' in selected_plugins and \ + 'feat' in selected_plugins and host_machine.endian() == 'little' + print_test_env = environment() + print_test_env.prepend('PYTHONPATH', meson.project_source_root()) + print_test_env.set('NVME_TEST_JSON', json_c_dep.found() ? '1' : '0') + test('nvme-cli - print-cli', + python3, + args: [files('nvme_print_test.py'), nvme_exe, mock_nvme_lib], + env: print_test_env, + depends: [nvme_exe, mock_nvme_lib], + timeout: 300) + endif + if 'sed' in selected_plugins and want_sed and \ conf.get('NVME_HAVE_SED_OPAL') != 0 sed_ioctls = [] diff --git a/tests/cli/nvme_print_test.py b/tests/cli/nvme_print_test.py new file mode 100644 index 0000000000..12bca9a8a2 --- /dev/null +++ b/tests/cli/nvme_print_test.py @@ -0,0 +1,336 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# +# This file is part of nvme-cli. +# Copyright (c) 2026 Dell Technologies Inc. or its subsidiaries. +# +# Authors: Martin Belanger +"""Run the read-only commands in every output format. + +Uses LD_PRELOAD mocking (see libmock_nvme.c), so no NVMe hardware and no +root are needed. The mock device answers every command with pseudo-random +data from a fixed seed, so each run sees the same data. Random data sets +most flags and fields, so the print code takes many of its branches. + +The fields that give the size of a log or the number of its entries are +set to small values (see _FIXUPS). With random values there, a command +would read gigabytes, or print entries that are not in the buffer. + +Each command must exit without a crash, and its JSON output must parse. + +Usage: python3 nvme_print_test.py +""" +import json +import os +import random +import shutil +import struct +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +from nvme_mock_ipc import (MockIPCServer, make_mock_env, + resolve_mock_lib_path, run_nvme) + +_NVME_BIN = (sys.argv[1] if len(sys.argv) > 1 and not sys.argv[1].startswith('-') + else 'nvme') +_MOCK_LIB = resolve_mock_lib_path("./libmock_nvme.so") + +_SEEDS = (1, 2) + +# Without json-c, some commands ignore '-o json' and print text. +_HAVE_JSON = os.environ.get('NVME_TEST_JSON', '1') == '1' + +_OPC_GET_LOG_PAGE = 0x02 +_OPC_IDENTIFY = 0x06 +_OPC_IO_MGMT_RECV = 0x12 +_OPC_RESV_REPORT = 0x0e +_OPC_DIR_RECV = 0x1a +_DIR_STREAMS_STATUS = 0x0102 # directive type 1, operation 2 +_OPC_GET_FEATURES = 0x0a +_FID_FDP_EVENTS = 0x1e +_LID_PERSISTENT_EVENT = 0x0d +_LID_SUPPORTED_CAP_CONFIG = 0x11 +_CNS_CTRL = 0x01 +_NVME_IOCTL_ID = 0x40 + + +def _put(buf, fmt, offset, value): + if offset + struct.calcsize(fmt) <= len(buf): + struct.pack_into(fmt, buf, offset, value) + + +# Log page ID -> (format, offset, value) fields to set in the log header. +# A value of None means the length of the first read, which is the header. +_FIXUPS = { + 0x0c: [('