On 11/30/2015 02:52 PM, Daniel Borkmann wrote:
On 11/30/2015 01:59 AM, Alexei Starovoitov wrote:
[...]
For large map->value_size the user space can trigger memory allocation warnings
like:
[...]
To avoid never succeeding kmalloc with order >= MAX_ORDER check that
elem->value_size and computed elem_size are within limits for both hash and
array type maps.
[...]
Large value_size can cause integer overflows in elem_size and map.pages
formulas, so check for that as well.
[...]
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 3f4c99e06c6b..b1e53b79c586 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -28,11 +28,17 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
attr->value_size == 0)
return ERR_PTR(-EINVAL);
+ if (attr->value_size >= 1 << (KMALLOC_SHIFT_MAX - 1))
+ /* if value_size is bigger, the user space won't be able to
+ * access the elements.
+ */
+ return ERR_PTR(-E2BIG);
+
Bit confused, given that in array map, we try kzalloc() with __GFP_NOWARN
already
and if that fails, we fall back to vzalloc(), it shouldn't trigger memory
allocation
warnings here ...
Then, integer overflow in elem_size with round_up(attr->value_size, 8) could
only
result in 0, which is already tested below.
elem_size = round_up(attr->value_size, 8);
/* check round_up into zero and u32 overflow */
if (elem_size == 0 ||
- attr->max_entries > (U32_MAX - sizeof(*array)) / elem_size)
+ attr->max_entries > (U32_MAX - PAGE_SIZE - sizeof(*array)) / elem_size)
return ERR_PTR(-ENOMEM);
... and this change seems to be needed for the integer overflow in map.pages?
So if the first check above intends to check for some size overflow (?), how is
it
then related to KMALLOC_SHIFT_MAX?
Ok, I see. The check and comment is related to the fact that when we do bpf(2)
syscall to lookup an element:
We call map_lookup_elem(), which does kmalloc() on the value_size.
So an individual entry lookup could fail with kmalloc() there, unrelated to an
individual map implementation.
Hmm, seems this patch fixes many things at once, maybe makes sense to split it?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
