[PATCH 0/6] add hardening checks to cmdline and cfgfile libs

[PATCH 0/6] add hardening checks to cmdline and cfgfile libs

[Bruce Richardson]

Using AI tools to review the cmdline and cfgfile libraries throws up a
couple of places in the libraries where additional hardening could help
prevent future issues. A number of these are purely defensive, e.g.
adding NULL checks to input parameters where a well-behaved app should
never call the function with a NULL value, and so those are not
explicitly marked for backport.

Bruce Richardson (6):
  cfgfile: add null checks to public APIs
  cfgfile: prevent issues with overflow on resize
  cmdline: harden parser result buffer handling
  cmdline: add explicit help function for bool type
  cmdline: guard zero-size destination buffers
  cmdline: add null checks for invalid input

 lib/cfgfile/rte_cfgfile.c          | 118 ++++++++++++++++++++---------
 lib/cmdline/cmdline.c              |   3 +
 lib/cmdline/cmdline_parse.c        |   6 +-
 lib/cmdline/cmdline_parse_bool.c   |  37 ++++++++-
 lib/cmdline/cmdline_parse_num.c    |   2 +-
 lib/cmdline/cmdline_parse_string.c |   3 +
 lib/cmdline/cmdline_rdline.c       |   3 +
 7 files changed, 128 insertions(+), 44 deletions(-)

--
2.51.0

[PATCH 1/6] cfgfile: add null checks to public APIs

[Bruce Richardson]

For safety, add NULL checks to each of the public APIs to avoid crashes
in the library. Even though NULL values are likely symptoms of a wider
problem, this is not a datapath library so there is no harm in taking a
few cycles for additional parameter checking.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/cfgfile/rte_cfgfile.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/lib/cfgfile/rte_cfgfile.c b/lib/cfgfile/rte_cfgfile.c
index c495bdf6ae..25fc792274 100644
--- a/lib/cfgfile/rte_cfgfile.c
+++ b/lib/cfgfile/rte_cfgfile.c
@@ -198,6 +198,10 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 		return NULL;
 
 	cfg = rte_cfgfile_create(flags);
+	if (cfg == NULL) {
+		fclose(f);
+		return NULL;
+	}
 
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
 		char *pos;
@@ -506,6 +510,9 @@ rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sectionname,
 	int num_sections = 0;
 	int i;
 
+	if (cfg == NULL)
+		return -1;
+
 	if (sectionname == NULL)
 		return cfg->num_sections;
 
@@ -523,8 +530,14 @@ rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 {
 	int i;
 
-	for (i = 0; i < cfg->num_sections && i < max_sections; i++)
+	if (cfg == NULL || sections == NULL || max_sections < 0)
+		return -1;
+
+	for (i = 0; i < cfg->num_sections && i < max_sections; i++) {
+		if (sections[i] == NULL)
+			return -1;
 		strlcpy(sections[i], cfg->sections[i].name, CFG_NAME_LEN);
+	}
 
 	return i;
 }
@@ -533,6 +546,9 @@ RTE_EXPORT_SYMBOL(rte_cfgfile_has_section)
 int
 rte_cfgfile_has_section(struct rte_cfgfile *cfg, const char *sectionname)
 {
+	if (cfg == NULL || sectionname == NULL)
+		return 0;
+
 	return _get_section(cfg, sectionname) != NULL;
 }
 
@@ -541,6 +557,9 @@ int
 rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 	const char *sectionname)
 {
+	if (cfg == NULL || sectionname == NULL)
+		return -1;
+
 	const struct rte_cfgfile_section *s = _get_section(cfg, sectionname);
 	if (s == NULL)
 		return -1;
@@ -552,6 +571,9 @@ int
 rte_cfgfile_section_num_entries_by_index(struct rte_cfgfile *cfg,
 	char *sectionname, int index)
 {
+	if (cfg == NULL || sectionname == NULL)
+		return -1;
+
 	if (index < 0 || index >= cfg->num_sections)
 		return -1;
 
@@ -566,6 +588,10 @@ rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const char *sectionname,
 		struct rte_cfgfile_entry *entries, int max_entries)
 {
 	int i;
+
+	if (cfg == NULL || sectionname == NULL || entries == NULL)
+		return -1;
+
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return -1;
@@ -583,6 +609,9 @@ rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg, int index,
 	int i;
 	const struct rte_cfgfile_section *sect;
 
+	if (cfg == NULL || sectionname == NULL || entries == NULL)
+		return -1;
+
 	if (index < 0 || index >= cfg->num_sections)
 		return -1;
 	sect = &cfg->sections[index];
@@ -598,6 +627,10 @@ rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname)
 {
 	int i;
+
+	if (cfg == NULL || sectionname == NULL || entryname == NULL)
+		return NULL;
+
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return NULL;
-- 
2.51.0

[PATCH 2/6] cfgfile: prevent issues with overflow on resize

[Bruce Richardson]

When resizing a cfgfile object to store more sections or entries, the
multiplication in the realloc call could lead to overflow and hence an
incorrect/smaller size being allocated. Prevent this by tightening up
sizing in the library:

- use size_t for sizes rather than int, avoiding negative values
- explicitly limit the number of entries to INT_MAX < SIZE_MAX, ensuring
  that all int indexes from the API will work.
- add range checks on allocation before multiplication, to avoid
  overflow.
  - This means a lower max entry count on 32-bit to avoid 32-bit
    allocation overflow.

Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/cfgfile/rte_cfgfile.c | 87 ++++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 38 deletions(-)

diff --git a/lib/cfgfile/rte_cfgfile.c b/lib/cfgfile/rte_cfgfile.c
index 25fc792274..040351ab4d 100644
--- a/lib/cfgfile/rte_cfgfile.c
+++ b/lib/cfgfile/rte_cfgfile.c
@@ -9,6 +9,7 @@
 #include <ctype.h>
 #include <errno.h>
 #include <limits.h>
+#include <stdint.h>
 
 #ifndef LINE_MAX
 #define LINE_MAX 2048
@@ -23,15 +24,15 @@
 
 struct rte_cfgfile_section {
 	char name[CFG_NAME_LEN];
-	int num_entries;
-	int allocated_entries;
+	size_t num_entries;
+	size_t allocated_entries;
 	struct rte_cfgfile_entry *entries;
 };
 
 struct rte_cfgfile {
 	int flags;
-	int num_sections;
-	int allocated_sections;
+	size_t num_sections;
+	size_t allocated_sections;
 	struct rte_cfgfile_section *sections;
 };
 
@@ -43,12 +44,27 @@ RTE_LOG_REGISTER_DEFAULT(cfgfile_logtype, INFO);
 	RTE_LOG_LINE_PREFIX(level, CFGFILE, "%s(): ", __func__, __VA_ARGS__)
 /* >8 End of setting up dynamic logging */
 
+/** define a max allocation limit for entry and section types
+ * for 64-bit systems, this is based on INT_MAX since APIs all work on int values.
+ * For 32-bit systems, this is based on SIZE_MAX / sizeof(type) to prevent overflow on allocation.
+ */
+#define CFG_ALLOC_MAX(type) \
+	(sizeof(int) == sizeof(size_t) ? (SIZE_MAX / sizeof(type)) : ((size_t)INT_MAX))
+
 /** when we resize a file structure, how many extra entries
  * for new sections do we add in */
 #define CFG_ALLOC_SECTION_BATCH 8
+/** max number of section entries we can possibly have.
+ * Used to prevent overflow on allocation. Based on INT_MAX since APIs all work on int values
+ */
+#define CFG_ALLOC_SECTION_MAX CFG_ALLOC_MAX(struct rte_cfgfile_section)
 /** when we resize a section structure, how many extra entries
  * for new entries do we add in */
 #define CFG_ALLOC_ENTRY_BATCH 16
+/** max number of data entries we can possibly have.
+ * Used to prevent overflow on allocation. Based on INT_MAX since APIs all work on int values
+ */
+#define CFG_ALLOC_ENTRY_MAX CFG_ALLOC_MAX(struct rte_cfgfile_entry)
 
 /**
  * Default cfgfile load parameters.
@@ -99,9 +115,7 @@ _strip(char *str, unsigned len)
 static struct rte_cfgfile_section *
 _get_section(struct rte_cfgfile *cfg, const char *sectionname)
 {
-	int i;
-
-	for (i = 0; i < cfg->num_sections; i++) {
+	for (size_t i = 0; i < cfg->num_sections; i++) {
 		if (strncmp(cfg->sections[i].name, sectionname,
 				sizeof(cfg->sections[0].name)) == 0)
 			return &cfg->sections[i];
@@ -118,6 +132,9 @@ _add_entry(struct rte_cfgfile_section *section, const char *entryname,
 
 	/* resize entry structure if we don't have room for more entries */
 	if (section->num_entries == section->allocated_entries) {
+		if (section->allocated_entries > CFG_ALLOC_ENTRY_MAX - CFG_ALLOC_ENTRY_BATCH)
+			return -ENOMEM;
+
 		struct rte_cfgfile_entry *n_entries = realloc(
 				section->entries,
 				sizeof(struct rte_cfgfile_entry) *
@@ -305,7 +322,6 @@ RTE_EXPORT_SYMBOL(rte_cfgfile_create)
 struct rte_cfgfile *
 rte_cfgfile_create(int flags)
 {
-	int i;
 	struct rte_cfgfile *cfg;
 
 	/* future proof flags usage */
@@ -328,7 +344,7 @@ rte_cfgfile_create(int flags)
 
 	cfg->allocated_sections = CFG_ALLOC_SECTION_BATCH;
 
-	for (i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
+	for (size_t i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
 		cfg->sections[i].entries = calloc(CFG_ALLOC_ENTRY_BATCH,
 					  sizeof(struct rte_cfgfile_entry));
 
@@ -345,7 +361,7 @@ rte_cfgfile_create(int flags)
 	return cfg;
 error1:
 	if (cfg->sections != NULL) {
-		for (i = 0; i < cfg->allocated_sections; i++) {
+		for (size_t i = 0; i < cfg->allocated_sections; i++) {
 			if (cfg->sections[i].entries != NULL) {
 				free(cfg->sections[i].entries);
 				cfg->sections[i].entries = NULL;
@@ -362,8 +378,6 @@ RTE_EXPORT_SYMBOL(rte_cfgfile_add_section)
 int
 rte_cfgfile_add_section(struct rte_cfgfile *cfg, const char *sectionname)
 {
-	int i;
-
 	if (cfg == NULL)
 		return -EINVAL;
 
@@ -375,6 +389,8 @@ rte_cfgfile_add_section(struct rte_cfgfile *cfg, const char *sectionname)
 
 	/* resize overall struct if we don't have room for more	sections */
 	if (cfg->num_sections == cfg->allocated_sections) {
+		if (cfg->allocated_sections > CFG_ALLOC_SECTION_MAX - CFG_ALLOC_SECTION_BATCH)
+			return -ENOMEM;
 
 		struct rte_cfgfile_section *n_sections =
 				realloc(cfg->sections,
@@ -385,7 +401,7 @@ rte_cfgfile_add_section(struct rte_cfgfile *cfg, const char *sectionname)
 		if (n_sections == NULL)
 			return -ENOMEM;
 
-		for (i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
+		for (size_t i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
 			n_sections[i + cfg->allocated_sections].num_entries = 0;
 			n_sections[i +
 				 cfg->allocated_sections].allocated_entries = 0;
@@ -428,8 +444,6 @@ RTE_EXPORT_SYMBOL(rte_cfgfile_set_entry)
 int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname, const char *entryvalue)
 {
-	int i;
-
 	if ((cfg == NULL) || (sectionname == NULL) || (entryname == NULL))
 		return -EINVAL;
 
@@ -442,7 +456,7 @@ int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 	if (entryvalue == NULL)
 		entryvalue = "";
 
-	for (i = 0; i < curr_section->num_entries; i++)
+	for (size_t i = 0; i < curr_section->num_entries; i++)
 		if (!strcmp(curr_section->entries[i].name, entryname)) {
 			strlcpy(curr_section->entries[i].value, entryvalue,
 				sizeof(curr_section->entries[i].value));
@@ -456,8 +470,6 @@ int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 RTE_EXPORT_SYMBOL(rte_cfgfile_save)
 int rte_cfgfile_save(struct rte_cfgfile *cfg, const char *filename)
 {
-	int i, j;
-
 	if ((cfg == NULL) || (filename == NULL))
 		return -EINVAL;
 
@@ -466,10 +478,10 @@ int rte_cfgfile_save(struct rte_cfgfile *cfg, const char *filename)
 	if (f == NULL)
 		return -EINVAL;
 
-	for (i = 0; i < cfg->num_sections; i++) {
+	for (size_t i = 0; i < cfg->num_sections; i++) {
 		fprintf(f, "[%s]\n", cfg->sections[i].name);
 
-		for (j = 0; j < cfg->sections[i].num_entries; j++) {
+		for (size_t j = 0; j < cfg->sections[i].num_entries; j++) {
 			fprintf(f, "%s=%s\n",
 					cfg->sections[i].entries[j].name,
 					cfg->sections[i].entries[j].value);
@@ -481,13 +493,11 @@ int rte_cfgfile_save(struct rte_cfgfile *cfg, const char *filename)
 RTE_EXPORT_SYMBOL(rte_cfgfile_close)
 int rte_cfgfile_close(struct rte_cfgfile *cfg)
 {
-	int i;
-
 	if (cfg == NULL)
 		return -1;
 
 	if (cfg->sections != NULL) {
-		for (i = 0; i < cfg->allocated_sections; i++) {
+		for (size_t i = 0; i < cfg->allocated_sections; i++) {
 			if (cfg->sections[i].entries != NULL) {
 				free(cfg->sections[i].entries);
 				cfg->sections[i].entries = NULL;
@@ -507,20 +517,20 @@ int
 rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sectionname,
 	size_t length)
 {
-	int num_sections = 0;
-	int i;
+	size_t num_sections = 0;
 
 	if (cfg == NULL)
 		return -1;
 
 	if (sectionname == NULL)
-		return cfg->num_sections;
+		return (int)cfg->num_sections;
 
-	for (i = 0; i < cfg->num_sections; i++) {
+	for (size_t i = 0; i < cfg->num_sections; i++) {
 		if (strncmp(cfg->sections[i].name, sectionname, length) == 0)
 			num_sections++;
 	}
-	return num_sections;
+
+	return (int)num_sections;
 }
 
 RTE_EXPORT_SYMBOL(rte_cfgfile_sections)
@@ -533,7 +543,7 @@ rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 	if (cfg == NULL || sections == NULL || max_sections < 0)
 		return -1;
 
-	for (i = 0; i < cfg->num_sections && i < max_sections; i++) {
+	for (i = 0; (size_t)i < cfg->num_sections && i < max_sections; i++) {
 		if (sections[i] == NULL)
 			return -1;
 		strlcpy(sections[i], cfg->sections[i].name, CFG_NAME_LEN);
@@ -563,7 +573,8 @@ rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 	const struct rte_cfgfile_section *s = _get_section(cfg, sectionname);
 	if (s == NULL)
 		return -1;
-	return s->num_entries;
+
+	return (int)s->num_entries;
 }
 
 RTE_EXPORT_SYMBOL(rte_cfgfile_section_num_entries_by_index)
@@ -574,13 +585,13 @@ rte_cfgfile_section_num_entries_by_index(struct rte_cfgfile *cfg,
 	if (cfg == NULL || sectionname == NULL)
 		return -1;
 
-	if (index < 0 || index >= cfg->num_sections)
+	if (index < 0 || (size_t)index >= cfg->num_sections)
 		return -1;
 
 	const struct rte_cfgfile_section *sect = &(cfg->sections[index]);
 
 	strlcpy(sectionname, sect->name, CFG_NAME_LEN);
-	return sect->num_entries;
+	return (int)sect->num_entries;
 }
 RTE_EXPORT_SYMBOL(rte_cfgfile_section_entries)
 int
@@ -595,7 +606,7 @@ rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const char *sectionname,
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return -1;
-	for (i = 0; i < max_entries && i < sect->num_entries; i++)
+	for (i = 0; i < max_entries && (size_t)i < sect->num_entries; i++)
 		entries[i] = sect->entries[i];
 	return i;
 }
@@ -611,12 +622,14 @@ rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg, int index,
 
 	if (cfg == NULL || sectionname == NULL || entries == NULL)
 		return -1;
+	if (max_entries < 0)
+		return -1;
 
-	if (index < 0 || index >= cfg->num_sections)
+	if (index < 0 || (size_t)index >= cfg->num_sections)
 		return -1;
 	sect = &cfg->sections[index];
 	strlcpy(sectionname, sect->name, CFG_NAME_LEN);
-	for (i = 0; i < max_entries && i < sect->num_entries; i++)
+	for (i = 0; i < max_entries && (size_t)i < sect->num_entries; i++)
 		entries[i] = sect->entries[i];
 	return i;
 }
@@ -626,15 +639,13 @@ const char *
 rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname)
 {
-	int i;
-
 	if (cfg == NULL || sectionname == NULL || entryname == NULL)
 		return NULL;
 
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return NULL;
-	for (i = 0; i < sect->num_entries; i++)
+	for (size_t i = 0; i < sect->num_entries; i++)
 		if (strncmp(sect->entries[i].name, entryname, CFG_NAME_LEN)
 									== 0)
 			return sect->entries[i].value;
-- 
2.51.0

[PATCH 3/6] cmdline: harden parser result buffer handling

[Bruce Richardson]

The cmdline parser had a few result-buffer safety gaps.

In boolean token parsing, the parser could write through a NULL output
pointer in parse-only paths (for example completion/match checks).  Add
proper output-pointer and output-size checks before storing the parsed
value.

In instruction matching, reject token offsets that are equal to the
result buffer size, not only greater than it, so tokens are never parsed
with a zero-sized output window at the end of the buffer.

In completion formatting, handle truncated strlcpy() output before
appending help text, preventing offset/size misuse when the destination
buffer is small.

Fixes: 985465997b73 ("ethdev: add xstats API to enable/disable counter")
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
Note: the first fixes line, though strange, is valid. The cmdline
library bool handling was added as part of the ethdev commit.
---
 lib/cmdline/cmdline_parse.c      |  6 ++++--
 lib/cmdline/cmdline_parse_bool.c | 19 ++++++++++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/cmdline/cmdline_parse.c b/lib/cmdline/cmdline_parse.c
index 201fddb8c3..d55c8db19d 100644
--- a/lib/cmdline/cmdline_parse.c
+++ b/lib/cmdline/cmdline_parse.c
@@ -133,7 +133,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
 		} else {
 			unsigned rb_sz;
 
-			if (token_hdr.offset > resbuf_size) {
+			if (token_hdr.offset >= resbuf_size) {
 				printf("Parse error(%s:%d): Token offset(%u) "
 					"exceeds maximum size(%u)\n",
 					__FILE__, __LINE__,
@@ -519,7 +519,9 @@ cmdline_complete(struct cmdline *cl, const char *buf, int *state,
 				}
 				(*state)++;
 				l=strlcpy(dst, tmpbuf, size);
-				if (l>=0 && token_hdr.ops->get_help) {
+				if ((unsigned int)l >= size)
+					return 1;
+				if (token_hdr.ops->get_help) {
 					token_hdr.ops->get_help(token_p, tmpbuf,
 								sizeof(tmpbuf));
 					help_str = inst->help_str;
diff --git a/lib/cmdline/cmdline_parse_bool.c b/lib/cmdline/cmdline_parse_bool.c
index e03cc3d545..a3f7adab58 100644
--- a/lib/cmdline/cmdline_parse_bool.c
+++ b/lib/cmdline/cmdline_parse_bool.c
@@ -35,17 +35,30 @@ static cmdline_parse_token_string_t cmd_parse_token_bool = {
 /* parse string to bool */
 int
 cmdline_parse_bool(__rte_unused cmdline_parse_token_hdr_t *tk, const char *srcbuf, void *res,
-	__rte_unused unsigned int ressize)
+	unsigned int ressize)
 {
 	cmdline_fixed_string_t on_off = {0};
+	uint8_t val;
+
+	if (!srcbuf || !*srcbuf)
+		return -1;
+
+	if (res != NULL && ressize < sizeof(uint8_t))
+		return -1;
+
 	if (cmdline_token_string_ops.parse
 			(&cmd_parse_token_bool.hdr, srcbuf, on_off, sizeof(on_off)) < 0)
 		return -1;
 
 	if (strcmp((char *)on_off, "on") == 0)
-		*(uint8_t *)res = 1;
+		val = 1;
 	else if (strcmp((char *)on_off, "off") == 0)
-		*(uint8_t *)res = 0;
+		val = 0;
+	else
+		return -1;
+
+	if (res != NULL)
+		*(uint8_t *)res = val;
 
 	return strlen(on_off);
 }
-- 
2.51.0

[PATCH 4/6] cmdline: add explicit help function for bool type

[Bruce Richardson]

Rather than using the string help output, have a specific boolean help
output that prints out on|off as the options.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/cmdline/cmdline_parse_bool.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline/cmdline_parse_bool.c b/lib/cmdline/cmdline_parse_bool.c
index a3f7adab58..d5d5499531 100644
--- a/lib/cmdline/cmdline_parse_bool.c
+++ b/lib/cmdline/cmdline_parse_bool.c
@@ -13,13 +13,17 @@
 #include "cmdline_parse.h"
 #include "cmdline_parse_bool.h"
 
+static int
+cmdline_get_help_bool(cmdline_parse_token_hdr_t *tk, char *dstbuf,
+		unsigned int size);
+
 
 RTE_EXPORT_EXPERIMENTAL_SYMBOL(cmdline_token_bool_ops, 25.03)
 struct cmdline_token_ops cmdline_token_bool_ops = {
 	.parse = cmdline_parse_bool,
 	.complete_get_nb = NULL,
 	.complete_get_elt = NULL,
-	.get_help = cmdline_get_help_string,
+	.get_help = cmdline_get_help_bool,
 };
 
 static cmdline_parse_token_string_t cmd_parse_token_bool = {
@@ -32,6 +36,18 @@ static cmdline_parse_token_string_t cmd_parse_token_bool = {
 	}
 };
 
+/* get help for bool token */
+static int
+cmdline_get_help_bool(__rte_unused cmdline_parse_token_hdr_t *tk,
+		char *dstbuf, unsigned int size)
+{
+	if (dstbuf == NULL || size == 0)
+		return -1;
+
+	strlcpy(dstbuf, "on|off", size);
+	return 0;
+}
+
 /* parse string to bool */
 int
 cmdline_parse_bool(__rte_unused cmdline_parse_token_hdr_t *tk, const char *srcbuf, void *res,
-- 
2.51.0

[PATCH 5/6] cmdline: guard zero-size destination buffers

[Bruce Richardson]

Add missing zero-size destination checks in cmdline helper routines.
Always check for size == 0, before doing an assignment of '\0' to string
position of "size - 1".

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/cmdline/cmdline_parse_num.c    | 2 +-
 lib/cmdline/cmdline_parse_string.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline/cmdline_parse_num.c b/lib/cmdline/cmdline_parse_num.c
index f21796bedb..4cfc900391 100644
--- a/lib/cmdline/cmdline_parse_num.c
+++ b/lib/cmdline/cmdline_parse_num.c
@@ -323,7 +323,7 @@ cmdline_get_help_num(cmdline_parse_token_hdr_t *tk, char *dstbuf, unsigned int s
 	struct cmdline_token_num_data nd;
 	int ret;
 
-	if (!tk)
+	if (!tk || !dstbuf || size == 0)
 		return -1;
 
 	memcpy(&nd, &((struct cmdline_token_num *)tk)->num_data, sizeof(nd));
diff --git a/lib/cmdline/cmdline_parse_string.c b/lib/cmdline/cmdline_parse_string.c
index 731947159f..33cf89f84f 100644
--- a/lib/cmdline/cmdline_parse_string.c
+++ b/lib/cmdline/cmdline_parse_string.c
@@ -171,6 +171,9 @@ int cmdline_complete_get_elt_string(cmdline_parse_token_hdr_t *tk, int idx,
 	if (!tk || !dstbuf || idx < 0)
 		return -1;
 
+	if (size == 0)
+		return -1;
+
 	tk2 = (struct cmdline_token_string *)tk;
 	sd = &tk2->string_data;
 
-- 
2.51.0

[PATCH 6/6] cmdline: add null checks for invalid input

[Bruce Richardson]

To harden the public API, add some NULL checks before dereferencing
pointers. Properly written apps should never call these with NULL, but
to increase resilience, we'll add the checks.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/cmdline/cmdline.c        | 3 +++
 lib/cmdline/cmdline_rdline.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index d1003f0b8e..51fbc36cef 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -103,6 +103,9 @@ RTE_EXPORT_SYMBOL(cmdline_get_rdline)
 struct rdline*
 cmdline_get_rdline(struct cmdline *cl)
 {
+	if (!cl)
+		return NULL;
+
 	return &cl->rdl;
 }
 
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 0a5a399b32..15da285c8d 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -618,6 +618,9 @@ RTE_EXPORT_SYMBOL(rdline_get_history_buffer_size)
 size_t
 rdline_get_history_buffer_size(struct rdline *rdl)
 {
+	if (!rdl)
+		return 0;
+
 	return sizeof(rdl->history_buf);
 }
 
-- 
2.51.0

Re: [PATCH 6/6] cmdline: add null checks for invalid input

[Alex Michael]

pmd_buffer_scatter test suite failed on Ubuntu 24.04 LTS with an Intel XL710 40GbE NIC. Conversely it didn’t fail on Ubuntu 22.04 with an Intel E810 NIC, so the issue lies either with the OS (mbuf size or mempool configuration mismatch / buffer handling logic), the NIC (driver-level bug), the interaction between them or the test suite itself (as it should be unrelated to NULL conditionals).

Re: [PATCH 6/6] cmdline: add null checks for invalid input

[Bruce Richardson]

On Thu, May 07, 2026 at 06:25:25PM -0400, Alex Michael wrote:
> pmd_buffer_scatter test suite failed on Ubuntu 24.04 LTS with an Intel XL710 40GbE NIC. Conversely it didn’t fail on Ubuntu 22.04 with an Intel E810 NIC, so the issue lies either with the OS (mbuf size or mempool configuration mismatch / buffer handling logic), the NIC (driver-level bug), the interaction between them or the test suite itself (as it should be unrelated to NULL conditionals).

Agree. The test failure seem unrelated to this patchset.