Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: avoid gcc overflow warning in test_xdp_vlan.c #7002

Closed
wants to merge 2 commits into from
Closed

bpf: avoid gcc overflow warning in test_xdp_vlan.c #7002

wants to merge 2 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: avoid gcc overflow warning in test_xdp_vlan.c
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 911edc6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 0093670
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: cbe35ad
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 0d03a4d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: fcd1ed8
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 80c5a07
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@dafaust
bpf: avoid gcc overflow warning in test_xdp_vlan.c
2fee7a6 
This patch fixes an integer overflow warning raised by GCC in
xdp_prognum1 of progs/test_xdp_vlan.c:

  GCC-BPF  [test_maps] test_xdp_vlan.bpf.o
progs/test_xdp_vlan.c: In function 'xdp_prognum1':
progs/test_xdp_vlan.c:163:25: error: integer overflow in expression
 '(short int)(((__builtin_constant_p((int)vlan_hdr->h_vlan_TCI)) != 0
   ? (int)(short unsigned int)((short int)((int)vlan_hdr->h_vlan_TCI
   << 8 >> 8) << 8 | (short int)((int)vlan_hdr->h_vlan_TCI << 0 >> 8
   << 0)) & 61440 : (int)__builtin_bswap16(vlan_hdr->h_vlan_TCI)
   & 61440) << 8 >> 8) << 8' of type 'short int' results in '0' [-Werror=overflow]
  163 |                         bpf_htons((bpf_ntohs(vlan_hdr->h_vlan_TCI) & 0xf000)
      |                         ^~~~~~~~~

The problem lies with the expansion of the bpf_htons macro and the
expression passed into it.  The bpf_htons macro (and similarly the
bpf_ntohs macro) expand to a ternary operation using either
__builtin_bswap16 or ___bpf_swab16 to swap the bytes, depending on
whether the expression is constant.

For an expression, with 'value' as a u16, like:

  bpf_htons (value & 0xf000)

The entire (value & 0xf000) is 'x' in the expansion of ___bpf_swab16
and we get as one part of the expanded swab16:

  ((__u16)(value & 0xf000) << 8 >> 8 << 8

This will always evaluate to 0, which is intentional since this
subexpression deals with the byte guaranteed to be 0 by the mask.

However, GCC warns because the precise reason this always evaluates to 0
is an overflow.  Specifically, the plain 0xf000 in the expression is a
signed 32-bit integer, which causes 'value' to also be promoted to a
signed 32-bit integer, and the combination of the 8-bit left shift and
down-cast back to __u16 results in a signed overflow (really a 'warning:
overflow in conversion from int to __u16' which is propegated up through
the rest of the expression leading to the ultimate overflow warning
above), which is a valid warning despite being the intended result of
this code.

Clang does not warn on this case, likely because it performs constant
folding later in the compilation process relative to GCC.  It seems that
by the time clang does constant folding for this expression, the side of
the ternary with this overflow has already been discarded.

Fortunately, this warning is easily silenced by simply making the 0xf000
mask explicitly unsigned.  This has no impact on the result.

Signed-off-by: David Faust <david.faust@oracle.com>
Cc: jose.marchesi@oracle.com
Cc: cupertino.miranda@oracle.com
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 20a759d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851686
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=851686 irrelevant now. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/851686=>bpf-next branch May 13, 2024 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
accepted bpf-next V1-ci-pass V1
Projects
None yet
Milestone
No milestone
1 participant
@dafaust