Skip to content

tap: add missing verification for short frame #109

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.

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

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

jallisonciq
Copy link

kernel-selftest-rt-baseline-8.8.log
kernel-selftest-rt-8.8-patched.log
jira VULN-9072
cve CVE-2024-41090
commit-author Si-Wei Liu [email protected]
commit ed7f2af

The cited commit missed to check against the validity of the frame length in the tap_get_user_xdp() path, which could cause a corrupted skb to be sent downstack. Even before the skb is transmitted, the tap_get_user_xdp()-->skb_set_network_header() may assume the size is more than ETH_HLEN. Once transmitted, this could either cause out-of-bound access beyond the actual length, or confuse the underlayer with incorrect or inconsistent header length in the skb metadata.

In the alternative path, tap_get_user() already prohibits short frame which has the length less than Ethernet header size from being transmitted.

This is to drop any frame shorter than the Ethernet header size just like how tap_get_user() does.

CVE: CVE-2024-41090
Link: https://lore.kernel.org/netdev/[email protected]/ Fixes: 0efac27 ("tap: accept an array of XDP buffs through sendmsg()")
Cc: [email protected]
Signed-off-by: Si-Wei Liu [email protected]
Signed-off-by: Dongli Zhang [email protected]
Reviewed-by: Willem de Bruijn [email protected]
Reviewed-by: Paolo Abeni [email protected]
Reviewed-by: Jason Wang [email protected]
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Jakub Kicinski [email protected]
(cherry picked from commit ed7f2af)

jira VULN-9072
cve CVE-2024-41090
commit-author Si-Wei Liu <[email protected]>
commit ed7f2af

The cited commit missed to check against the validity of the frame length
in the tap_get_user_xdp() path, which could cause a corrupted skb to be
sent downstack. Even before the skb is transmitted, the
tap_get_user_xdp()-->skb_set_network_header() may assume the size is more
than ETH_HLEN. Once transmitted, this could either cause out-of-bound
access beyond the actual length, or confuse the underlayer with incorrect
or inconsistent header length in the skb metadata.

In the alternative path, tap_get_user() already prohibits short frame which
has the length less than Ethernet header size from being transmitted.

This is to drop any frame shorter than the Ethernet header size just like
how tap_get_user() does.

CVE: CVE-2024-41090
Link: https://lore.kernel.org/netdev/[email protected]/
Fixes: 0efac27 ("tap: accept an array of XDP buffs through sendmsg()")
	Cc: [email protected]
	Signed-off-by: Si-Wei Liu <[email protected]>
	Signed-off-by: Dongli Zhang <[email protected]>
	Reviewed-by: Willem de Bruijn <[email protected]>
	Reviewed-by: Paolo Abeni <[email protected]>
	Reviewed-by: Jason Wang <[email protected]>
Link: https://patch.msgid.link/[email protected]
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit ed7f2af)

Signed-off-by: Jeremy Allison <[email protected]>
Copy link

@gvrose8192 gvrose8192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to other commits in this series for CVE-2024-41090 - LGTM. Thanks!

@gvrose8192 gvrose8192 self-requested a review February 8, 2025 00:53
@gvrose8192
Copy link

I take back my approval - I missed the build log and kernel boot line with PREEMPT_RT?

We like to see a build and boot log. Thanks!

@jallisonciq
Copy link
Author

I take back my approval - I missed the build log and kernel boot line with PREEMPT_RT?

We like to see a build and boot log. Thanks!

OK. I didn't know I needed that for RT. I'll re-build for the build log and get a boot log (that's just the dmsg output yeah ?).

@gvrose8192
Copy link

gvrose8192 commented Feb 8, 2025

I take back my approval - I missed the build log and kernel boot line with PREEMPT_RT?
We like to see a build and boot log. Thanks!

OK. I didn't know I needed that for RT. I'll re-build for the build log and get a boot log (that's just the dmsg output yeah ?).

Thanks! I just run ../kernel-tools/kernel_build.sh skipkabi | tee > build.log for RT branches and after the build completes and reboots the VM you log in and run a simple 'uname -a' to show PREEMPT_RT. Oh, and post the beginning and end of the build log.

@jallisonciq
Copy link
Author

I take back my approval - I missed the build log and kernel boot line with PREEMPT_RT?
We like to see a build and boot log. Thanks!

OK. I didn't know I needed that for RT. I'll re-build for the build log and get a boot log (that's just the dmsg output yeah ?).

Thanks! I just run ../kernel-tools/kernel_build.sh skipkabi | tee > build.log for RT branches and after the build completes and reboots the VM you log in and run a simple 'uname -a' to show PREEMPT_RT. Oh, and post the beginning and end of the build log.

Arg. Got pulled into an openssl CVE. Might take me a little while to get to this (just letting you know I haven't forgotten about it).

@jallisonciq
Copy link
Author

kernel_build_log_8_8_rt.log

Output from uname -a

Linux r8_8_rt_lts 4.18.0-ciqlts8_8-rt+ #1 SMP PREEMPT_RT Fri Feb 7 21:52:38 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

@gvrose8192
Copy link

kernel_build_log_8_8_rt.log

Output from uname -a

Linux r8_8_rt_lts 4.18.0-ciqlts8_8-rt+ #1 SMP PREEMPT_RT Fri Feb 7 21:52:38 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

So long as you ran the kernel selftests under the PREEMPT_RT kernel, which is what i'm understanding to be the case, then approved.

Copy link

@gvrose8192 gvrose8192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jallisonciq
Copy link
Author

kernel_build_log_8_8_rt.log
Output from uname -a
Linux r8_8_rt_lts 4.18.0-ciqlts8_8-rt+ #1 SMP PREEMPT_RT Fri Feb 7 21:52:38 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

So long as you ran the kernel selftests under the PREEMPT_RT kernel, which is what i'm understanding to be the case, then approved.

Yes. I run the selftests first under the original kernel, then boot into the new kernel and run again under the patched kernel (both using PREEMPT_RT in this branch).
Thanks !

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥌

@jallisonciq jallisonciq merged commit 9945b8b into ciqlts8_8-rt Feb 11, 2025
1 check passed
@jallisonciq jallisonciq deleted the jra_8_8_rt_lts_CVE-2024-41090 branch February 11, 2025 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants