[PATCH net-next v6 5/8] net: pktgen: fix access outside of user given buffer in pktgen_if_write()

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 Reviewed-by: Simon Horman horms@kernel.org --- Changes v5 -> v6 - adjust to dropped patch ''net: pktgen: use defines for the various dec/hex number parsing digits lengths'

Changes v4 -> v5 - split up patchset into part i/ii (suggested by Simon Horman) - add rev-by Simon Horman

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 | 178 ++++++++++++++++++++++++++++++---------------- 1 file changed, 118 insertions(+), 60 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 03ea6b5db156..ae5e81e62733 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -842,10 +842,10 @@ static ssize_t strn_len(const char __user *user_buffer, size_t maxlen) * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example. */ static ssize_t get_imix_entries(const char __user *buffer, + size_t maxlen, struct pktgen_dev *pkt_dev) { - const size_t max_digits = 10; - size_t i = 0; + size_t i = 0, max; ssize_t len; char c;

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

- len = num_arg(&buffer[i], max_digits, &size); + max = min(10, 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 */ @@ -872,7 +875,8 @@ static ssize_t get_imix_entries(const char __user *buffer, if (size < 14 + 20 + 8) size = 14 + 20 + 8;

- len = num_arg(&buffer[i], max_digits, &weight); + max = min(10, maxlen - i); + len = num_arg(&buffer[i], max, &weight); if (len < 0) return len; if (weight <= 0) @@ -882,20 +886,23 @@ static ssize_t 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 ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev) +static ssize_t get_labels(const char __user *buffer, + size_t maxlen, struct pktgen_dev *pkt_dev) { unsigned int n = 0; - size_t i = 0; + size_t i = 0, max; ssize_t len; char c;

@@ -906,17 +913,20 @@ static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev) if (n >= MAX_MPLS_LABELS) return -E2BIG;

- len = hex32_arg(&buffer[i], 8, &tmp); + max = min(8, 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; @@ -981,8 +991,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;

@@ -1010,7 +1020,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1027,7 +1038,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1046,7 +1058,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], 10, &value); + max = min(10, count - i); + len = num_arg(&user_buffer[i], max, &value); if (len < 0) return len;

@@ -1066,7 +1079,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;

@@ -1077,7 +1091,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1088,7 +1103,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

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

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

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

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

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

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

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

@@ -1195,7 +1218,8 @@ static ssize_t pktgen_if_write(struct file *file, return count; } if (!strcmp(name, "clone_skb")) { - len = num_arg(&user_buffer[i], 10, &value); + max = min(10, 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 @@ -1216,7 +1240,8 @@ static ssize_t pktgen_if_write(struct file *file, return count; } if (!strcmp(name, "count")) { - len = num_arg(&user_buffer[i], 10, &value); + max = min(10, count - i); + len = num_arg(&user_buffer[i], max, &value); if (len < 0) return len;

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

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

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

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

@@ -1295,11 +1324,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; @@ -1335,11 +1365,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; @@ -1384,7 +1415,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;

@@ -1404,7 +1436,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;

@@ -1424,7 +1457,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;

@@ -1447,7 +1481,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;

@@ -1469,7 +1504,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;

@@ -1490,7 +1526,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;

@@ -1513,7 +1550,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;

@@ -1533,7 +1571,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;

@@ -1553,7 +1592,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;

@@ -1570,7 +1610,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;

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

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

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

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

@@ -1630,7 +1674,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1641,7 +1686,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1654,7 +1700,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; @@ -1675,7 +1722,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1702,7 +1750,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1717,7 +1766,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1732,7 +1782,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

@@ -1759,7 +1810,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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

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

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

@@ -1790,7 +1843,9 @@ static ssize_t pktgen_if_write(struct file *file,

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

@@ -1806,7 +1861,9 @@ static ssize_t pktgen_if_write(struct file *file,

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

@@ -1821,7 +1878,8 @@ static ssize_t pktgen_if_write(struct file *file, }

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