From: Siddharth Chintamaneni <sidchintamaneni@xxxxxx> This patch address a possible deadlock issue in queue and stack map types. Deadlock could happen when a nested BPF program acquires the same lock as the parent BPF program to perform a write operation on the same map as the first one. This bug is also reported by syzbot. Link: https://lore.kernel.org/lkml/0000000000004c3fc90615f37756@xxxxxxxxxx/ Reported-by: syzbot+8bdfc2c53fb2b63e1871@xxxxxxxxxxxxxxxxxxxxxxxxx Fixes: f1a2e44a3aec ("bpf: add queue and stack maps") Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@xxxxxx> --- kernel/bpf/queue_stack_maps.c | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c index d869f51ea93a..4b7df1a53cf2 100644 --- a/kernel/bpf/queue_stack_maps.c +++ b/kernel/bpf/queue_stack_maps.c @@ -18,6 +18,7 @@ struct bpf_queue_stack { raw_spinlock_t lock; u32 head, tail; u32 size; /* max_entries + 1 */ + int __percpu *map_locked; char elements[] __aligned(8); }; @@ -78,6 +79,16 @@ static struct bpf_map *queue_stack_map_alloc(union bpf_attr *attr) qs->size = size; + qs->map_locked = bpf_map_alloc_percpu(&qs->map, + sizeof(int), + sizeof(int), + GFP_USER); + if (!qs->map_locked) { + bpf_map_area_free(qs); + return ERR_PTR(-ENOMEM); + } + + raw_spin_lock_init(&qs->lock); return &qs->map; @@ -98,6 +109,16 @@ static long __queue_map_get(struct bpf_map *map, void *value, bool delete) int err = 0; void *ptr; + preempt_disable(); + local_irq_save(flags); + if (unlikely(__this_cpu_inc_return(*(qs->map_locked)) != 1)) { + __this_cpu_dec(*(qs->map_locked)); + local_irq_restore(flags); + preempt_enable(); + return -EBUSY; + } + preempt_enable(); + if (in_nmi()) { if (!raw_spin_trylock_irqsave(&qs->lock, flags)) return -EBUSY; @@ -133,6 +154,17 @@ static long __stack_map_get(struct bpf_map *map, void *value, bool delete) void *ptr; u32 index; + preempt_disable(); + local_irq_save(flags); + if (unlikely(__this_cpu_inc_return(*(qs->map_locked)) != 1)) { + __this_cpu_dec(*(qs->map_locked)); + local_irq_restore(flags); + preempt_enable(); + return -EBUSY; + } + preempt_enable(); + + if (in_nmi()) { if (!raw_spin_trylock_irqsave(&qs->lock, flags)) return -EBUSY; @@ -194,6 +226,16 @@ static long queue_stack_map_push_elem(struct bpf_map *map, void *value, int err = 0; void *dst; + preempt_disable(); + local_irq_save(irq_flags); + if (unlikely(__this_cpu_inc_return(*(qs->map_locked)) != 1)) { + __this_cpu_dec(*(qs->map_locked)); + local_irq_restore(irq_flags); + preempt_enable(); + return -EBUSY; + } + preempt_enable(); + /* BPF_EXIST is used to force making room for a new element in case the * map is full */ -- 2.44.0
