[PATCH net-next v4 13/17] net: pktgen: fix access outside of user given buffer in pktgen_if_write()

Peter Seiderer
2025-02-05T13:12:16Z

Honour the user given buffer size for the hex32_arg(), num_arg(),
strn_len(), get_imix_entries() and get_labels() calls (otherwise they will
access memory outside of the user given buffer).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4:
- replace C99 comment (suggested by Paolo Abeni)
- drop available characters check in strn_len() (suggested by Paolo Abeni)
- factored out patch 'net: pktgen: align some variable declarations to the
most common pattern' (suggested by Paolo Abeni)
- factored out patch 'net: pktgen: remove extra tmp variable (re-use len
instead)' (suggested by Paolo Abeni)
- factored out patch 'net: pktgen: remove some superfluous variable
initializing' (suggested by Paolo Abeni)
- factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
(suggested by Paolo Abeni)
- factored out 'net: pktgen: hex32_arg/num_arg error out in case no
characters are available' (suggested by Paolo Abeni)
- factored out 'net: pktgen: num_arg error out in case no valid character
is parsed' (suggested by Paolo Abeni)

Changes v2 -> v3:
- no changes

Changes v1 -> v2:
- additional fix get_imix_entries() and get_labels()
---
net/core/pktgen.c | 176 ++++++++++++++++++++++++++++++----------------
1 file changed, 117 insertions(+), 59 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 0fd15f21119b..3c42ecf17ba2 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -851,10 +851,11 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
* "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
*/
static int get_imix_entries(const char __user *buffer,
+ unsigned int maxlen,
struct pktgen_dev *pkt_dev)
{
char c;
- int i = 0, len;
+ int i = 0, max, len;

pkt_dev->n_imix_entries = 0;

@@ -865,10 +866,13 @@ static int get_imix_entries(const char __user *buffer,
if (pkt_dev->n_imix_entries >= MAX_IMIX_ENTRIES)
return -E2BIG;

- len = num_arg(&buffer[i], DEC_10_DIGITS, &size);
+ max = min(DEC_10_DIGITS, maxlen - i);
+ len = num_arg(&buffer[i], max, &size);
if (len < 0)
return len;
i += len;
+ if (i >= maxlen)
+ return -EINVAL;
if (get_user(c, &buffer[i]))
return -EFAULT;
/* Check for comma between size_i and weight_i */
@@ -879,7 +883,8 @@ static int get_imix_entries(const char __user *buffer,
if (size < 14 + 20 + 8)
size = 14 + 20 + 8;

- len = num_arg(&buffer[i], DEC_10_DIGITS, &weight);
+ max = min(DEC_10_DIGITS, maxlen - i);
+ len = num_arg(&buffer[i], max, &weight);
if (len < 0)
return len;
if (weight <= 0)
@@ -889,21 +894,23 @@ static int get_imix_entries(const char __user *buffer,
pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;

i += len;
+ pkt_dev->n_imix_entries++;
+
+ if (i >= maxlen)
+ break;
if (get_user(c, &buffer[i]))
return -EFAULT;
-
i++;
- pkt_dev->n_imix_entries++;
} while (c == ' ');

return i;
}

-static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
+static int get_labels(const char __user *buffer, int maxlen, struct pktgen_dev *pkt_dev)
{
char c;
- int i = 0, len;
unsigned int n = 0;
+ int i = 0, max, len;

pkt_dev->nr_labels = 0;
do {
@@ -912,17 +919,20 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
if (n >= MAX_MPLS_LABELS)
return -E2BIG;

- len = hex32_arg(&buffer[i], HEX_8_DIGITS, &tmp);
+ max = min(HEX_8_DIGITS, maxlen - i);
+ len = hex32_arg(&buffer[i], max, &tmp);
if (len <= 0)
return len;
pkt_dev->labels[n] = htonl(tmp);
if (pkt_dev->labels[n] & MPLS_STACK_BOTTOM)
pkt_dev->flags |= F_MPLS_RND;
i += len;
+ n++;
+ if (i >= maxlen)
+ break;
if (get_user(c, &buffer[i]))
return -EFAULT;
i++;
- n++;
} while (c == ',');

pkt_dev->nr_labels = n;
@@ -986,8 +996,8 @@ static ssize_t pktgen_if_write(struct file *file,
i = len;

/* Read variable name */
-
- len = strn_len(&user_buffer[i], sizeof(name) - 1);
+ max = min(sizeof(name) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1015,7 +1025,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "min_pkt_size")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1032,7 +1043,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "max_pkt_size")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1051,7 +1063,8 @@ static ssize_t pktgen_if_write(struct file *file,
/* Shortcut for min = max */

if (!strcmp(name, "pkt_size")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1071,7 +1084,8 @@ static ssize_t pktgen_if_write(struct file *file,
if (pkt_dev->clone_skb > 0)
return -EINVAL;

- len = get_imix_entries(&user_buffer[i], pkt_dev);
+ max = count - i;
+ len = get_imix_entries(&user_buffer[i], max, pkt_dev);
if (len < 0)
return len;

@@ -1082,7 +1096,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "debug")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1093,7 +1108,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "frags")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1103,7 +1119,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "delay")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1118,7 +1135,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "rate")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1133,7 +1151,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "ratep")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1148,7 +1167,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "udp_src_min")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1161,7 +1181,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "udp_dst_min")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1174,7 +1195,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "udp_src_max")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1187,7 +1209,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "udp_dst_max")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1200,7 +1223,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "clone_skb")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;
/* clone_skb is not supported for netif_receive xmit_mode and
@@ -1221,7 +1245,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "count")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1232,7 +1257,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "src_mac_count")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1246,7 +1272,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst_mac_count")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1260,7 +1287,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "burst")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1279,7 +1307,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "node")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1300,11 +1329,12 @@ static ssize_t pktgen_if_write(struct file *file,
if (!strcmp(name, "xmit_mode")) {
char f[32];

- memset(f, 0, 32);
- len = strn_len(&user_buffer[i], sizeof(f) - 1);
+ max = min(sizeof(f) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

+ memset(f, 0, sizeof(f));
if (copy_from_user(f, &user_buffer[i], len))
return -EFAULT;
i += len;
@@ -1340,11 +1370,12 @@ static ssize_t pktgen_if_write(struct file *file,
char f[32];
char *end;

- memset(f, 0, 32);
- len = strn_len(&user_buffer[i], sizeof(f) - 1);
+ max = min(sizeof(f) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

+ memset(f, 0, 32);
if (copy_from_user(f, &user_buffer[i], len))
return -EFAULT;
i += len;
@@ -1389,7 +1420,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
- len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
+ max = min(sizeof(pkt_dev->dst_min) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1409,7 +1441,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst_max")) {
- len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
+ max = min(sizeof(pkt_dev->dst_max) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1429,7 +1462,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst6")) {
- len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+ max = min(sizeof(buf) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1452,7 +1486,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst6_min")) {
- len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+ max = min(sizeof(buf) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1474,7 +1509,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst6_max")) {
- len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+ max = min(sizeof(buf) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1495,7 +1531,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "src6")) {
- len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+ max = min(sizeof(buf) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1518,7 +1555,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "src_min")) {
- len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
+ max = min(sizeof(pkt_dev->src_min) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1538,7 +1576,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "src_max")) {
- len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
+ max = min(sizeof(pkt_dev->src_max) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1558,7 +1597,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst_mac")) {
- len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+ max = min(sizeof(valstr) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1575,7 +1615,8 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "src_mac")) {
- len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+ max = min(sizeof(valstr) - 1, count - i);
+ len = strn_len(&user_buffer[i], max);
if (len < 0)
return len;

@@ -1599,7 +1640,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "flows")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1613,7 +1655,8 @@ static ssize_t pktgen_if_write(struct file *file,
}
#ifdef CONFIG_XFRM
if (!strcmp(name, "spi")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1624,7 +1667,8 @@ static ssize_t pktgen_if_write(struct file *file,
}
#endif
if (!strcmp(name, "flowlen")) {
- len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+ max = min(DEC_10_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1635,7 +1679,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "queue_map_min")) {
- len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
+ max = min(DEC_5_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1646,7 +1691,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "queue_map_max")) {
- len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
+ max = min(DEC_5_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1659,7 +1705,8 @@ static ssize_t pktgen_if_write(struct file *file,
if (!strcmp(name, "mpls")) {
unsigned int n, cnt;

- len = get_labels(&user_buffer[i], pkt_dev);
+ max = count - i;
+ len = get_labels(&user_buffer[i], max, pkt_dev);
if (len < 0)
return len;
i += len;
@@ -1680,7 +1727,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "vlan_id")) {
- len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
+ max = min(DEC_4_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1707,7 +1755,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "vlan_p")) {
- len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+ max = min(DEC_1_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1722,7 +1771,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "vlan_cfi")) {
- len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+ max = min(DEC_1_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1737,7 +1787,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "svlan_id")) {
- len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
+ max = min(DEC_4_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1764,7 +1815,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "svlan_p")) {
- len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+ max = min(DEC_1_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1779,7 +1831,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "svlan_cfi")) {
- len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+ max = min(DEC_1_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

@@ -1795,7 +1848,9 @@ static ssize_t pktgen_if_write(struct file *file,

if (!strcmp(name, "tos")) {
__u32 tmp_value;
- len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
+
+ max = min(HEX_2_DIGITS, count - i);
+ len = hex32_arg(&user_buffer[i], max, &tmp_value);
if (len < 0)
return len;

@@ -1811,7 +1866,9 @@ static ssize_t pktgen_if_write(struct file *file,

if (!strcmp(name, "traffic_class")) {
__u32 tmp_value;
- len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
+
+ max = min(HEX_2_DIGITS, count - i);
+ len = hex32_arg(&user_buffer[i], max, &tmp_value);
if (len < 0)
return len;

@@ -1826,7 +1883,8 @@ static ssize_t pktgen_if_write(struct file *file,
}

if (!strcmp(name, "skb_priority")) {
- len = num_arg(&user_buffer[i], DEC_9_DIGITS, &value);
+ max = min(DEC_9_DIGITS, count - i);
+ len = num_arg(&user_buffer[i], max, &value);
if (len < 0)
return len;

--
2.48.1