netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] devmap: use bpf_map_area_alloc() for allocating hash buckets
@ 2020-06-16 14:28 Toke Høiland-Jørgensen
  2020-06-17  6:06 ` John Fastabend
  0 siblings, 1 reply; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-06-16 14:28 UTC (permalink / raw)
  To: daniel, ast; +Cc: Toke Høiland-Jørgensen, bpf, netdev, Xiumei Mu

Syzkaller discovered that creating a hash of type devmap_hash with a large
number of entries can hit the memory allocator limit for allocating
contiguous memory regions. There's really no reason to use kmalloc_array()
directly in the devmap code, so just switch it to the existing
bpf_map_area_alloc() function that is used elsewhere.

Reported-by: Xiumei Mu <xmu@redhat.com>
Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 kernel/bpf/devmap.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 0cbb72cdaf63..5fdbc776a760 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -86,12 +86,13 @@ static DEFINE_PER_CPU(struct list_head, dev_flush_list);
 static DEFINE_SPINLOCK(dev_map_lock);
 static LIST_HEAD(dev_map_list);
 
-static struct hlist_head *dev_map_create_hash(unsigned int entries)
+static struct hlist_head *dev_map_create_hash(unsigned int entries,
+					      int numa_node)
 {
 	int i;
 	struct hlist_head *hash;
 
-	hash = kmalloc_array(entries, sizeof(*hash), GFP_KERNEL);
+	hash = bpf_map_area_alloc(entries * sizeof(*hash), numa_node);
 	if (hash != NULL)
 		for (i = 0; i < entries; i++)
 			INIT_HLIST_HEAD(&hash[i]);
@@ -145,7 +146,8 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
 		return -EINVAL;
 
 	if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
-		dtab->dev_index_head = dev_map_create_hash(dtab->n_buckets);
+		dtab->dev_index_head = dev_map_create_hash(dtab->n_buckets,
+							   dtab->map.numa_node);
 		if (!dtab->dev_index_head)
 			goto free_charge;
 
@@ -232,7 +234,7 @@ static void dev_map_free(struct bpf_map *map)
 			}
 		}
 
-		kfree(dtab->dev_index_head);
+		bpf_map_area_free(dtab->dev_index_head);
 	} else {
 		for (i = 0; i < dtab->map.max_entries; i++) {
 			struct bpf_dtab_netdev *dev;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH bpf] devmap: use bpf_map_area_alloc() for allocating hash buckets
  2020-06-16 14:28 [PATCH bpf] devmap: use bpf_map_area_alloc() for allocating hash buckets Toke Høiland-Jørgensen
@ 2020-06-17  6:06 ` John Fastabend
  2020-06-17 17:03   ` Alexei Starovoitov
  0 siblings, 1 reply; 3+ messages in thread
From: John Fastabend @ 2020-06-17  6:06 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, daniel, ast
  Cc: Toke Høiland-Jørgensen, bpf, netdev, Xiumei Mu

Toke Høiland-Jørgensen wrote:
> Syzkaller discovered that creating a hash of type devmap_hash with a large
> number of entries can hit the memory allocator limit for allocating
> contiguous memory regions. There's really no reason to use kmalloc_array()
> directly in the devmap code, so just switch it to the existing
> bpf_map_area_alloc() function that is used elsewhere.
> 
> Reported-by: Xiumei Mu <xmu@redhat.com>
> Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  kernel/bpf/devmap.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] devmap: use bpf_map_area_alloc() for allocating hash buckets
  2020-06-17  6:06 ` John Fastabend
@ 2020-06-17 17:03   ` Alexei Starovoitov
  0 siblings, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2020-06-17 17:03 UTC (permalink / raw)
  To: John Fastabend
  Cc: Toke Høiland-Jørgensen, Daniel Borkmann,
	Alexei Starovoitov, bpf, Network Development, Xiumei Mu

On Tue, Jun 16, 2020 at 11:07 PM John Fastabend
<john.fastabend@gmail.com> wrote:
>
> Toke Høiland-Jørgensen wrote:
> > Syzkaller discovered that creating a hash of type devmap_hash with a large
> > number of entries can hit the memory allocator limit for allocating
> > contiguous memory regions. There's really no reason to use kmalloc_array()
> > directly in the devmap code, so just switch it to the existing
> > bpf_map_area_alloc() function that is used elsewhere.
> >
> > Reported-by: Xiumei Mu <xmu@redhat.com>
> > Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > ---
> >  kernel/bpf/devmap.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Applied. Thanks

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-17 17:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 14:28 [PATCH bpf] devmap: use bpf_map_area_alloc() for allocating hash buckets Toke Høiland-Jørgensen
2020-06-17  6:06 ` John Fastabend
2020-06-17 17:03   ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).