2024-09-09 04:52 UTC+0800 ~ Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
On Sun, Sep 08, 2024 at 08:48:40PM +0100, Quentin Monnet wrote:
On 08/09/2024 15:00, Kuan-Wei Chiu wrote:
Replace shifts of '1' with '1U' in bitwise operations within
__show_dev_tc_bpf() to prevent undefined behavior caused by shifting
into the sign bit of a signed integer. By using '1U', the operations
are explicitly performed on unsigned integers, avoiding potential
integer overflow or sign-related issues.
Signed-off-by: Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
Looks good, thank you.
Acked-by: Quentin Monnet <qmo@xxxxxxxxxx>
How did you find these?
TL;DR: I discovered this issue through code review.
I am a student developer trying to contribute to the Linux kernel. I
was attempting to compile bpftool with ubsan enabled, and while running
./bpftool net list, I encountered the following error message:
net.c:827:2: runtime error: null pointer passed as argument 1, which is declared to never be null
This prompted me to review the code in net.c, and during that process,
I unexpectedly came across the bug that this patch addresses.
Nice
As for the ubsan complaint mentioned above, it was triggered because
qsort is being called as qsort(NULL, 0, ...) when netfilter has no
entries to display. In glibc, qsort is marked with __nonnull ((1, 4)).
However, I found conflicting information on cppreference.com [1], which
states that when count is zero, both ptr and comp can be NULL. This
confused me, so I will need to check the C standard to clarify this. If
it turns out that qsort(NULL, 0, ...) is invalid, I will submit a
separate patch to fix it.
OK, thanks for looking into it!
BTW, should this patch include a Fixes tag and a Cc @stable?
We could maybe have used a Fixes:, but the patch is merged already so
never mind. As for stable, I don't think this is necessary. I don't
believe we can hit the undefined behaviour today; and we encourage
people to package bpftool from the GitHub mirror anyway, where your
patch will land soon.
Thanks,
Quentin
