[an error occurred while processing this directive]
lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
[an error occurred while processing this directive]
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <85762e7f-49d3-44ea-b23f-4f258959470b@huaweicloud.com>
Date: Thu, 30 Oct 2025 20:03:06 +0800
From: Xu Kuohai <xukuohai@...weicloud.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
 <eddyz87@...il.com>, Yonghong Song <yhs@...com>, Song Liu <song@...nel.org>
Subject: Re: [PATCH bpf-next v3 1/3] bpf: Add overwrite mode for BPF ring
 buffer

On 10/28/2025 10:47 AM, Andrii Nakryiko wrote:
> On Fri, Oct 17, 2025 at 9:04 PM Xu Kuohai <xukuohai@...weicloud.com> wrote:


[...]

> 
>> @@ -72,6 +73,8 @@ struct bpf_ringbuf {
>>           */
>>          unsigned long consumer_pos __aligned(PAGE_SIZE);
>>          unsigned long producer_pos __aligned(PAGE_SIZE);
>> +       /* points to the record right after the last overwritten one */
>> +       unsigned long overwrite_pos;
> 
> I moved this after pending_pos, as all these fields are actually
> exposed to the user space, so didn't want to unnecessarily shift
> pending_pos.

OK, that makes sense

> 
>>          unsigned long pending_pos;
>>          char data[] __aligned(PAGE_SIZE);
>>   };
>> @@ -166,7 +169,7 @@ static void bpf_ringbuf_notify(struct irq_work *work)
>>    * considering that the maximum value of data_sz is (4GB - 1), there
>>    * will be no overflow, so just note the size limit in the comments.
>>    */
>> -static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node)
>> +static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node, bool overwrite_mode)
>>   {
>>          struct bpf_ringbuf *rb;
>>
>> @@ -183,17 +186,25 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node)
>>          rb->consumer_pos = 0;
>>          rb->producer_pos = 0;
>>          rb->pending_pos = 0;
>> +       rb->overwrite_mode = overwrite_mode;
>>
>>          return rb;
>>   }
>>
>>   static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
>>   {
>> +       bool overwrite_mode = false;
>>          struct bpf_ringbuf_map *rb_map;
>>
>>          if (attr->map_flags & ~RINGBUF_CREATE_FLAG_MASK)
>>                  return ERR_PTR(-EINVAL);
>>
>> +       if (attr->map_flags & BPF_F_RB_OVERWRITE) {
>> +               if (attr->map_type == BPF_MAP_TYPE_USER_RINGBUF)
> 
> this seemed error prone if we ever add another ringbuf type (unlikely,
> but still), so I inverted this all to allow BPF_F_RB_OVERWRITE only
> for BPF_MAP_TYPE_RINGBUF. We should try to be as strict as possible by
> default.
>

Got it, thanks.

>> +                       return ERR_PTR(-EINVAL);
>> +               overwrite_mode = true;
>> +       }
>> +
>>          if (attr->key_size || attr->value_size ||
>>              !is_power_of_2(attr->max_entries) ||
>>              !PAGE_ALIGNED(attr->max_entries))
>> @@ -205,7 +216,7 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
>>
>>          bpf_map_init_from_attr(&rb_map->map, attr);
>>
>> -       rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node);
>> +       rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node, overwrite_mode);
>>          if (!rb_map->rb) {
>>                  bpf_map_area_free(rb_map);
>>                  return ERR_PTR(-ENOMEM);
>> @@ -293,13 +304,25 @@ static int ringbuf_map_mmap_user(struct bpf_map *map, struct vm_area_struct *vma
>>          return remap_vmalloc_range(vma, rb_map->rb, vma->vm_pgoff + RINGBUF_PGOFF);
>>   }
>>
>> +/* Return an estimate of the available data in the ring buffer.
> 
> Fixed up comment style
> 
> [...]
> 
>>   static u32 ringbuf_total_data_sz(const struct bpf_ringbuf *rb)
>> @@ -402,11 +425,41 @@ bpf_ringbuf_restore_from_rec(struct bpf_ringbuf_hdr *hdr)
>>          return (void*)((addr & PAGE_MASK) - off);
>>   }
>>
>> +static bool bpf_ringbuf_has_space(const struct bpf_ringbuf *rb,
>> +                                 unsigned long new_prod_pos,
>> +                                 unsigned long cons_pos,
>> +                                 unsigned long pend_pos)
>> +{
>> +       /* no space if oldest not yet committed record until the newest
>> +        * record span more than (ringbuf_size - 1).
>> +        */
> 
> same, keep in mind that we now use kernel-wide comment style with /*
> on separate line. Fixed up all other places as well.
>

Thanks for fixing everything! I think I should update my editor config, it’s
still stuck at an 80-character line width.

>> +       if (new_prod_pos - pend_pos > rb->mask)
>> +               return false;
>> +
>> +       /* ok, we have space in overwrite mode */
>> +       if (unlikely(rb->overwrite_mode))
>> +               return true;
>> +
>> +       /* no space if producer position advances more than (ringbuf_size - 1)
>> +        * ahead of consumer position when not in overwrite mode.
>> +        */
>> +       if (new_prod_pos - cons_pos > rb->mask)
>> +               return false;
>> +
>> +       return true;
>> +}
>> +
> 
> [...]