[LTS 8.6] net: mdio: fix undefined behavior in bit shift for __mdiobus_register #359
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jira VULN-66411 cve CVE-2022-49907 commit-author Gaosheng Cui <[email protected]> commit 40e4eb3 Shifting signed 32-bit value by 31 bits is undefined, so changing significant bit to unsigned. The UBSAN warning calltrace like below: UBSAN: shift-out-of-bounds in drivers/net/phy/mdio_bus.c:586:27 left shift of 1 by 31 places cannot be represented in type 'int' Call Trace: <TASK> dump_stack_lvl+0x7d/0xa5 dump_stack+0x15/0x1b ubsan_epilogue+0xe/0x4e __ubsan_handle_shift_out_of_bounds+0x1e7/0x20c __mdiobus_register+0x49d/0x4e0 fixed_mdio_bus_init+0xd8/0x12d do_one_initcall+0x76/0x430 kernel_init_freeable+0x3b3/0x422 kernel_init+0x24/0x1e0 ret_from_fork+0x1f/0x30 </TASK> Fixes: 4fd5f81 ("phylib: allow incremental scanning of an mii bus") Signed-off-by: Gaosheng Cui <[email protected]> Reviewed-by: Andrew Lunn <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> (cherry picked from commit 40e4eb3) Signed-off-by: Marcin Wcisło <[email protected]>
pvts-mat
changed the title
thefossguy-ciq
requested review from
jainanmol84,
bmastbergen and
thefossguy-ciq
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[LTS 8.6]
CVE-2022-49907
VULN-66411
Problem
https://www.cve.org/CVERecord?id=CVE-2022-49907
Applicability: yes (similar as in #358)
The bug applies to LTS 8.6: the affected MDIO bus driver is central to the control of any ethernet interface device. The patch 40e4eb3 is not backported onto LTS 8.6, yet it is backported onto its mainline sibling stable 4.19 in a3fafc9.
Solution (same as in #358)
The solution in 40e4eb3 involves using the
BIT(i)macro instead of the raw bit shift1 << ito obtain anintwith i -th bit set. The fully expandedBIT(i)macro boils down to1UL << iconstruct operating on unsigned type where the left shit is defined for the full range of the type's bits (seeinclude/vdso/bits.h,include/uapi/linux/const.h,include/linux/bits.h).kABI check: passed
Boot test: passed
boot-test.log
Kselftests: passed relative
Coverage
All the network-related tests (except the unstable ones):
net/forwarding(exceptsch_tbf_ets.sh,sch_tbf_root.sh,tc_actions.sh,mirror_gre_vlan_bridge_1q.sh,sch_tbf_prio.sh,sch_ets.sh,ipip_hier_gre_keys.sh,mirror_gre_bridge_1d_vlan.sh),net/mptcp(exceptsimult_flows.sh),net(exceptreuseaddr_conflict,reuseport_addr_any.sh,xfrm_policy.sh,ip_defrag.sh,gro.sh,udpgro_fwd.sh,txtimestamp.sh,udpgso_bench.sh),netfilter(exceptnft_trans_stress.sh)Reference
kselftests–ciqlts8_6–run1.log
Patch
kselftests–ciqlts8_6-CVE-2022-49907–run1.log
Comparison
The tests results for patch and reference are the same
Specific tests: could not replicate (similar as in #358)
An attempt was made to replicate the bug by compiling the kernel with
CONFIG_UBSAN=y. Unfortunately, the integer overflows resulting from bit shifts were not being captured by UBSAN, not only in the affected driver, but in general, as could have been demonstrated with the modifiedtest_ubsanmodule (enabled withCONFIG_TEST_UBSAN=y) where, along the originaltest_ubsan_shift_out_of_boundsfunction:an additional
test_ubsan_shift_out_of_bounds1was defined testing the exact situation as described in the CVE:The added
test_ubsan_shift_out_of_bounds1test was ignored by UBSAN at runtime (as well as original tests for {add, sub, mul, negate} overflows), which only kicked in during the division by zero testtest_ubsan_divrem_overflowand the kernel rebooted immediately after:After this unsuccessful replication attempt the specific testing efforts were then abandoned.