Skip to content

[fips-9] net/sched: [cls_route, cls_fw, cls_u32] : No longer copy tcf_result on update to avoid use-after-free #205

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

Conversation

bmastbergen
Copy link
Collaborator

jira VULN-34673
cve CVE-2023-4128

This vuln was created for CVE-2023-4128, however this CVE was actually superseded by three individual CVEs: CVE-2023-4206, CVE-2023-4207, and CVE-2023-4208. In the commit messages I used this vuln ticket but labelled the three commits with CVE-2023-420*. Hopefully that makes sense. If I should do something different, just let me know.

Build

build.log

Testing

kselftests were run before and after applying the changes:

selftest-before.log

selftest-after.log

brett@lycia ~/ciq/vuln-34673 % grep ^ok selftest-before.log | wc -l
304
brett@lycia ~/ciq/vuln-34673 % grep ^ok selftest-after.log | wc -l
306
brett@lycia ~/ciq/vuln-34673 %

Copy link

@thefossguy-ciq thefossguy-ciq left a comment

Choose a reason for hiding this comment

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

🚤

@PlaidCat
Copy link
Collaborator

I think we should look at adding the actual VULN for the CVE that way it better mapps

So the system does support multiple jira and CVE tags, they just need to be their own line.
Example:

net/sched: cls_route: No longer copy tcf_result on update to avoid use-after-free

jira VULN-34673
cve https://github.com/advisories/GHSA-r8pm-459q-x6hw
commit-author valis <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/b80b829e9e2c1b3f7aae34855e04d8f6ecaf13c8

becomes if we want to put both CVEs in it, i'm not sure that we do since as you said the 3 individuals superseded the one

net/sched: cls_route: No longer copy tcf_result on update to avoid use-after-free

jira VULN-34673
jira VULN-8844
cve CVE-2023-4206
cve CVE-2023-4218
commit-author valis <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/b80b829e9e2c1b3f7aae34855e04d8f6ecaf13c8

@bmastbergen
Copy link
Collaborator Author

I think we should look at adding the actual VULN for the CVE that way it better mapps

So the system does support multiple jira and CVE tags, they just need to be their own line. Example:

net/sched: cls_route: No longer copy tcf_result on update to avoid use-after-free

jira VULN-34673
cve https://github.com/advisories/GHSA-r8pm-459q-x6hw
commit-author valis <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/b80b829e9e2c1b3f7aae34855e04d8f6ecaf13c8

becomes if we want to put both CVEs in it, i'm not sure that we do since as you said the 3 individuals superseded the one

net/sched: cls_route: No longer copy tcf_result on update to avoid use-after-free

jira VULN-34673
jira VULN-8844
cve CVE-2023-4206
cve CVE-2023-4218
commit-author valis <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/b80b829e9e2c1b3f7aae34855e04d8f6ecaf13c8

Ok, cool. Will add the additional VULN and CVE references. Thanks!

…e-after-free

jira VULN-34673
jira VULN-8844
cve CVE-2023-4206
cve CVE-2023-4218
commit-author valis <[email protected]>
commit b80b829

When route4_change() is called on an existing filter, the whole
tcf_result struct is always copied into the new instance of the filter.

This causes a problem when updating a filter bound to a class,
as tcf_unbind_filter() is always called on the old instance in the
success path, decreasing filter_cnt of the still referenced class
and allowing it to be deleted, leading to a use-after-free.

Fix this by no longer copying the tcf_result struct from the old filter.

Fixes: 1109c00 ("net: sched: RCU cls_route")
	Reported-by: valis <[email protected]>
	Reported-by: Bing-Jhong Billy Jheng <[email protected]>
	Signed-off-by: valis <[email protected]>
	Signed-off-by: Jamal Hadi Salim <[email protected]>
	Reviewed-by: Victor Nogueira <[email protected]>
	Reviewed-by: Pedro Tammela <[email protected]>
	Reviewed-by: M A Ramdhan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit b80b829)
	Signed-off-by: Brett Mastbergen <[email protected]>
…fter-free

jira VULN-34673
jira VULN-8842
cve CVE-2023-4207
cve CVE-2023-4218
commit-author valis <[email protected]>
commit 76e42ae

When fw_change() is called on an existing filter, the whole
tcf_result struct is always copied into the new instance of the filter.

This causes a problem when updating a filter bound to a class,
as tcf_unbind_filter() is always called on the old instance in the
success path, decreasing filter_cnt of the still referenced class
and allowing it to be deleted, leading to a use-after-free.

Fix this by no longer copying the tcf_result struct from the old filter.

Fixes: e35a8ee ("net: sched: fw use RCU")
	Reported-by: valis <[email protected]>
	Reported-by: Bing-Jhong Billy Jheng <[email protected]>
	Signed-off-by: valis <[email protected]>
	Signed-off-by: Jamal Hadi Salim <[email protected]>
	Reviewed-by: Victor Nogueira <[email protected]>
	Reviewed-by: Pedro Tammela <[email protected]>
	Reviewed-by: M A Ramdhan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit 76e42ae)
	Signed-off-by: Brett Mastbergen <[email protected]>
…after-free

jira VULN-34673
jira VULN-8840
cve CVE-2023-4208
cve CVE-2023-4218
commit-author valis <[email protected]>
commit 3044b16

When u32_change() is called on an existing filter, the whole
tcf_result struct is always copied into the new instance of the filter.

This causes a problem when updating a filter bound to a class,
as tcf_unbind_filter() is always called on the old instance in the
success path, decreasing filter_cnt of the still referenced class
and allowing it to be deleted, leading to a use-after-free.

Fix this by no longer copying the tcf_result struct from the old filter.

Fixes: de5df63 ("net: sched: cls_u32 changes to knode must appear atomic to readers")
	Reported-by: valis <[email protected]>
	Reported-by: M A Ramdhan <[email protected]>
	Signed-off-by: valis <[email protected]>
	Signed-off-by: Jamal Hadi Salim <[email protected]>
	Reviewed-by: Victor Nogueira <[email protected]>
	Reviewed-by: Pedro Tammela <[email protected]>
	Reviewed-by: M A Ramdhan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit 3044b16)
	Signed-off-by: Brett Mastbergen <[email protected]>
@bmastbergen bmastbergen force-pushed the bmastbergen_fips-9-compliant/5.14.0-284.30.1/VULN-34673 branch from ad03f30 to 70ad667 Compare April 14, 2025 14:34
@bmastbergen
Copy link
Collaborator Author

Ok, force pushed my 3 commits with the additional VULN and CVE tags

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.

:shipit:

@PlaidCat
Copy link
Collaborator

Ok, force pushed my 3 commits with the additional VULN and CVE tags

Thank You

@bmastbergen bmastbergen merged commit e8a9008 into fips-9-compliant/5.14.0-284.30.1 Apr 15, 2025
3 checks passed
@bmastbergen bmastbergen deleted the bmastbergen_fips-9-compliant/5.14.0-284.30.1/VULN-34673 branch April 15, 2025 19: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