From b80d8fa5109f8e31296436e05e26b9e81c81850a Mon Sep 17 00:00:00 2001 From: Prabhakar Pujeri Date: Tue, 25 Aug 2026 14:32:33 +0530 Subject: [PATCH] libnvme: nbft: reject heap objects too small for the structures they decode The SSNS extended info descriptor already discloses and validates its length, but the HFI trinfo object is still decoded into a fixed-size struct nbft_hfi_info_tcp without any minimum-length check, and the SSNS transport address heap object is fed unguarded to format_ip_addr(), which always reads 16 bytes regardless of the declared object length. The newer HFI ext-info object has the same under-declared length trap. Give __get_heap_obj() a min_len parameter rejecting non-string objects shorter than the structure the caller dereferences. The minimum is derived from the output pointer type via _Generic in the get_heap_obj() macro (strings and byte arrays keep min 0); call sites drop their (char **) casts. _Generic is a C11 feature thus update the project settings. C11 is 15 years old, so should be fair to ask for such a compiler. Signed-off-by: Prabhakar Pujeri [wagi: added C11 dependency] Signed-off-by: Daniel Wagner --- NEWS.md | 5 ++++ libnvme/src/nvme/nbft.c | 57 ++++++++++++++++++++++++++++++++--------- meson.build | 2 +- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/NEWS.md b/NEWS.md index 533eb653ef..09261c6a60 100644 --- a/NEWS.md +++ b/NEWS.md @@ -177,6 +177,11 @@ instead of alongside current commands, and running one prints a warning that it will be removed in the next major version. +* The project now builds with `-std=gnu11` instead of `-std=gnu99`, + raising the minimum required compiler support from C99 to C11. A + toolchain new enough for C11 is now required to build nvme-cli and + libnvme. + * `nvme disconnect-all` with no options no longer disconnects every fabric controller. It now only disconnects controllers with no recorded owner in the new ownership registry. A diff --git a/libnvme/src/nvme/nbft.c b/libnvme/src/nvme/nbft.c index ef31d38c37..1b9dc8f043 100644 --- a/libnvme/src/nvme/nbft.c +++ b/libnvme/src/nvme/nbft.c @@ -111,7 +111,7 @@ static int __get_heap_obj(struct libnvme_global_ctx *ctx, struct nbft_header *header, const char *filename, const char *descriptorname, const char *fieldname, struct nbft_heap_obj obj, bool is_string, - char **output, __u16 *length) + size_t min_len, char **output, __u16 *length) { __u16 obj_length = le16_to_cpu(obj.length); @@ -128,6 +128,14 @@ static int __get_heap_obj(struct libnvme_global_ctx *ctx, return -EINVAL; } + if (!is_string && obj_length < min_len) { + libnvme_msg(ctx, LIBNVME_LOG_DEBUG, + "file %s: object '%s' in descriptor '%s' is too short (%d, expected %zu)\n", + filename, fieldname, descriptorname, + obj_length, min_len); + return -EINVAL; + } + /* check that string is zero terminated correctly */ *output = (char *)header + le32_to_cpu(obj.offset); @@ -150,16 +158,31 @@ static int __get_heap_obj(struct libnvme_global_ctx *ctx, return 0; } -#define get_heap_obj(ctx, descriptor, obj, is_string, output) \ - __get_heap_obj(ctx, header, nbft->filename, \ - stringify(descriptor), stringify(obj), \ - descriptor->obj, is_string, \ - output, NULL) +/* + * Heap objects with structured (non-string) content are dereferenced as a + * struct by the caller, so make sure the object is at least as large as the + * structure it is interpreted as. String and plain byte-array objects + * have no minimum. The SSNS extended-info reader performs its own + * spec-length validation, so it is exempt here too. + */ +#define get_heap_obj(ctx, descriptor, obj, is_string, output) \ + __get_heap_obj(ctx, header, nbft->filename, \ + stringify(descriptor), stringify(obj), \ + descriptor->obj, is_string, \ + _Generic((output), \ + char **: 0, \ + __u8 **: 0, \ + struct nbft_hfi_info_tcp **: \ + sizeof(**(output)), \ + struct nbft_hfi_info_ext **: \ + sizeof(**(output)), \ + struct nbft_ssns_ext_info **: 0), \ + (char **)(output), NULL) #define get_heap_obj_len(ctx, descriptor, obj, is_string, output, length) \ __get_heap_obj(ctx, header, nbft->filename, \ stringify(descriptor), stringify(obj), \ - descriptor->obj, is_string, output, length) + descriptor->obj, is_string, 0, output, length) static struct libnbft_discovery *discovery_from_index(struct libnbft_info *nbft, int i) @@ -275,10 +298,20 @@ static int read_ssns(struct libnvme_global_ctx *ctx, } /* subsystem transport address */ - ret = get_heap_obj(ctx, raw_ssns, subsys_traddr_obj, 0, (char **)&tmp); + ret = get_heap_obj(ctx, raw_ssns, subsys_traddr_obj, 0, &tmp); if (ret) goto fail; + /* format_ip_addr() always reads a full 16 bytes of IP address */ + if (le16_to_cpu(raw_ssns->subsys_traddr_obj.length) < sizeof(struct in6_addr)) { + libnvme_msg(ctx, LIBNVME_LOG_DEBUG, + "file %s: SSNS %d transport address heap object too short (%d bytes)\n", + nbft->filename, ssns->index, + le16_to_cpu(raw_ssns->subsys_traddr_obj.length)); + ret = -EINVAL; + goto fail; + } + format_ip_addr(ssns->traddr, sizeof(ssns->traddr), tmp); /* subsystem transport service identifier */ @@ -314,7 +347,7 @@ static int read_ssns(struct libnvme_global_ctx *ctx, /* HFI descriptors */ ret = get_heap_obj(ctx, raw_ssns, secondary_hfi_assoc_obj, - 0, (char **)&ss_hfi_indexes); + 0, &ss_hfi_indexes); if (ret) goto fail; @@ -383,7 +416,7 @@ static int read_ssns(struct libnvme_global_ctx *ctx, struct nbft_ssns_ext_info *ssns_extended_info; if (!get_heap_obj(ctx, raw_ssns, ssns_extended_info_desc_obj, - 0, (char **)&ssns_extended_info)) { + 0, &ssns_extended_info)) { read_ssns_exended_info(ctx, nbft, ssns, ssns_extended_info, le16_to_cpu(raw_ssns->ssns_extended_info_desc_obj.length)); @@ -479,7 +512,7 @@ static int read_hfi_info_tcp(struct libnvme_global_ctx *ctx, hfi->tcp_info.pcie_seg_num = raw_hfi_info_tcp->pcie_seg_num; if (!get_heap_obj(ctx, raw_hfi_info_tcp, hfi_ext_info_obj, - 0, (char **)&hfi_ext_info)) + 0, &hfi_ext_info)) read_hfi_info_dhcp(ctx, nbft, hfi_ext_info, hfi); } @@ -517,7 +550,7 @@ static int read_hfi(struct libnvme_global_ctx *ctx, struct libnbft_info *nbft, hfi->transport[sizeof(hfi->transport) - 1] = '\0'; ret = get_heap_obj(ctx, raw_hfi, trinfo_obj, - 0, (char **)&raw_hfi_info_tcp); + 0, &raw_hfi_info_tcp); if (ret) goto fail; diff --git a/meson.build b/meson.build index efc701e87b..53e78f62bb 100644 --- a/meson.build +++ b/meson.build @@ -21,7 +21,7 @@ project( ], version: '3.0-rc2', default_options: [ - 'c_std=gnu99', + 'c_std=gnu11', 'buildtype=debugoptimized', 'warning_level=1', 'sysconfdir=etc',