On 8/25/23 5:17 AM, Hou Tao wrote:
On 8/25/2023 6:09 AM, Andrew Werner wrote:
Before this patch, the producer position could overflow `unsigned
long`, in which case libbpf would forever stop processing new writes to
the ringbuf. Similarly, overflows of the producer position could result
in __bpf_user_ringbuf_peek not discovering available data. This patch
addresses that bug by computing using the signed delta between the
consumer and producer position to determine if data is available; the
delta computation is robust to overflow.
A more defensive check could be to ensure that the delta is within
the allowed range, but such defensive checks are neither present in
the kernel side code nor in libbpf. The overflow that this patch
handles can occur while the producer and consumer follow a correct
protocol.
Secondarily, the type used to represent the positions in the
user_ring_buffer functions in both libbpf and the kernel has been
changed from u64 to unsigned long to match the type used in the
kernel's representation of the structure. The change occurs in the
same patch because it's required to align the data availability
calculations between the userspace producing ringbuf and the bpf
producing ringbuf.
Because the changes include both the change for ring buffer and user
ring buffer. I think it is better to split the changes into three
patches to ease the backports of these changes: one patch for change in
libbpf for ring buffer, and another two patches for changes in libbpf
and kernel for user ring buffer.
Splitting off the kernel parts into a separate patch would indeed be
good so that stable team can pull it. (Pls also add a Fixes tag.)
Thanks,
Daniel
