Skip to content

tun: add missing verification for short frame #182

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
Mar 31, 2025

Conversation

jallisonciq
Copy link

@jallisonciq jallisonciq commented Mar 27, 2025

baseline_selftest9.2.txt
patched_selftest9.2.txt

uname -a
Linux r92lts 5.14.0-ciqlts9_2+ #1 SMP PREEMPT_DYNAMIC Tue Mar 25 21:53:08 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

jira VULN-8274
cve CVE-2024-41091
commit-author Dongli Zhang [email protected] commit 0495848

The cited commit missed to check against the validity of the frame length in the tun_xdp_one() path, which could cause a corrupted skb to be sent downstack. Even before the skb is transmitted, the tun_xdp_one-->eth_type_trans() may access the Ethernet header although it can be less 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, tun_get_user() already prohibits short frame which has the length less than Ethernet header size from being transmitted for IFF_TAP.

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

CVE: CVE-2024-41091
Inspired-by: https://lore.kernel.org/netdev/[email protected]/ Fixes: 043d222 ("tuntap: accept an array of XDP buffs through sendmsg()")
Cc: [email protected]
Signed-off-by: Dongli Zhang [email protected]
Reviewed-by: Si-Wei Liu [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 0495848)
Signed-off-by: Jeremy Allison [email protected]

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.

🥌

Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

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

The ticket listed is for fip-9.2 the pull request is against ciqlts9_2
For ciqlts9_2 we need VULN-8274

@jallisonciq
Copy link
Author

Thanks for catching that. Sorry for the mistake. I'll fix asap.

jira VULN-8274
cve CVE-2024-41091
commit-author Dongli Zhang <[email protected]>
commit 0495848

The cited commit missed to check against the validity of the frame length
in the tun_xdp_one() path, which could cause a corrupted skb to be sent
downstack. Even before the skb is transmitted, the
tun_xdp_one-->eth_type_trans() may access the Ethernet header although it
can be less 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, tun_get_user() already prohibits short frame which
has the length less than Ethernet header size from being transmitted for
IFF_TAP.

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

CVE: CVE-2024-41091
Inspired-by: https://lore.kernel.org/netdev/[email protected]/
Fixes: 043d222 ("tuntap: accept an array of XDP buffs through sendmsg()")
	Cc: [email protected]
	Signed-off-by: Dongli Zhang <[email protected]>
	Reviewed-by: Si-Wei Liu <[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 0495848)
	Signed-off-by: Jeremy Allison <[email protected]>
@jallisonciq jallisonciq force-pushed the {jallison}_ciqlts9_2_CVE-2024-41091 branch from 5b89e79 to 4e6e62e Compare March 31, 2025 18:21
@jallisonciq
Copy link
Author

Updated comment and commit message. Sorry about that.

Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

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

Updated comment and commit message. Sorry about that.

No worries its on the long list of things I would like to see automation do for us.

:shipit:

@jallisonciq jallisonciq merged commit cd74e4f into ciqlts9_2 Mar 31, 2025
3 checks passed
@jallisonciq jallisonciq deleted the {jallison}_ciqlts9_2_CVE-2024-41091 branch March 31, 2025 21:16
github-actions bot pushed a commit to bmastbergen/kernel-src-tree that referenced this pull request Apr 11, 2025
Commit 8284066 ("ublk: grab request reference when the request is handled
by userspace") doesn't grab request reference in case of recovery reissue.
Then the request can be requeued & re-dispatch & failed when canceling
uring command.

If it is one zc request, the request can be freed before io_uring
returns the zc buffer back, then cause kernel panic:

[  126.773061] BUG: kernel NULL pointer dereference, address: 00000000000000c8
[  126.773657] #PF: supervisor read access in kernel mode
[  126.774052] #PF: error_code(0x0000) - not-present page
[  126.774455] PGD 0 P4D 0
[  126.774698] Oops: Oops: 0000 [#1] SMP NOPTI
[  126.775034] CPU: 13 UID: 0 PID: 1612 Comm: kworker/u64:55 Not tainted 6.14.0_blk+ ctrliq#182 PREEMPT(full)
[  126.775676] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39 04/01/2014
[  126.776275] Workqueue: iou_exit io_ring_exit_work
[  126.776651] RIP: 0010:ublk_io_release+0x14/0x130 [ublk_drv]

Fixes it by always grabbing request reference for aborting the request.

Reported-by: Caleb Sander Mateos <[email protected]>
Closes: https://lore.kernel.org/linux-block/CADUfDZodKfOGUeWrnAxcZiLT+puaZX8jDHoj_sfHZCOZwhzz6A@mail.gmail.com/
Fixes: 8284066 ("ublk: grab request reference when the request is handled by userspace")
Signed-off-by: Ming Lei <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
github-actions bot pushed a commit to bmastbergen/kernel-src-tree that referenced this pull request Apr 20, 2025
[ Upstream commit 6ee6bd5 ]

Commit 8284066 ("ublk: grab request reference when the request is handled
by userspace") doesn't grab request reference in case of recovery reissue.
Then the request can be requeued & re-dispatch & failed when canceling
uring command.

If it is one zc request, the request can be freed before io_uring
returns the zc buffer back, then cause kernel panic:

[  126.773061] BUG: kernel NULL pointer dereference, address: 00000000000000c8
[  126.773657] #PF: supervisor read access in kernel mode
[  126.774052] #PF: error_code(0x0000) - not-present page
[  126.774455] PGD 0 P4D 0
[  126.774698] Oops: Oops: 0000 [#1] SMP NOPTI
[  126.775034] CPU: 13 UID: 0 PID: 1612 Comm: kworker/u64:55 Not tainted 6.14.0_blk+ ctrliq#182 PREEMPT(full)
[  126.775676] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39 04/01/2014
[  126.776275] Workqueue: iou_exit io_ring_exit_work
[  126.776651] RIP: 0010:ublk_io_release+0x14/0x130 [ublk_drv]

Fixes it by always grabbing request reference for aborting the request.

Reported-by: Caleb Sander Mateos <[email protected]>
Closes: https://lore.kernel.org/linux-block/CADUfDZodKfOGUeWrnAxcZiLT+puaZX8jDHoj_sfHZCOZwhzz6A@mail.gmail.com/
Fixes: 8284066 ("ublk: grab request reference when the request is handled by userspace")
Signed-off-by: Ming Lei <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
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