On 8/25/23 6:39 PM, Andrew Werner wrote:
On Fri, Aug 25, 2023 at 11:28 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
On 8/25/23 12: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
Hm, but won't this mismatch for 64bit kernel and 32bit user space? Why
not fixate both on u64 instead so types are consistent?
Sure. It feels like if we do that then we'd break existing 32bit
big-endian clients, though I am not sure those exist. Concretely, the
request here would be to change the kernel structure and all library
usages to use u64, right?
Yes, to align all consistently on u64. From your diff, I read that for
the kernel its the case already.
Thanks,
Daniel
