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.

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

Fix: Potential Vulnerability in Cloned Function #2118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tabudz
Copy link

@tabudz tabudz commented Feb 20, 2025

Description
This PR fixes a security vulnerability in address_space_translate_for_iotlb() that was cloned from qemu but did not receive the security patch. The original issue was reported and fixed under qemu/qemu@418ade7.
This PR applies the same patch to eliminate the vulnerability.

References
https://nvd.nist.gov/vuln/detail/CVE-2022-35414
qemu/qemu@418ade7

The bug is an uninitialized memory read, along the translate_fail
path, which results in garbage being read from iotlb_to_section,
which can lead to a crash in io_readx/io_writex.

The bug may be fixed by writing any value with zero
in ~TARGET_PAGE_MASK, so that the call to iotlb_to_section using
the xlat'ed address returns io_mem_unassigned, as desired by the
translate_fail path.

It is most useful to record the original physical page address,
which will eventually be logged by memory_region_access_valid
when the access is rejected by unassigned_mem_accepts.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1065
Signed-off-by: Richard Henderson <[email protected]>
Reviewed-by: Peter Maydell <[email protected]>
Message-Id: <[email protected]>
@wtdcode
Copy link
Member

wtdcode commented Feb 20, 2025

The backport is failing CI. Could you adapt it?

@tabudz
Copy link
Author

tabudz commented Feb 20, 2025

What do you think went wrong here? I applied the same fix as in the original commit.

@wtdcode
Copy link
Member

wtdcode commented Feb 21, 2025

TARGET_PAGE_MASK is not constant in Unicorn and you need to adapt it accordingly.

@tabudz
Copy link
Author

tabudz commented Feb 21, 2025

But TARGET_PAGE_MASK appears multiple times in the same file, doesn't it?

@PhilippTakacs
Copy link
Contributor

Yes but TARGET_PAGE_MASK is defined to "uc->init_target_page->mask" and in this function uc is not defined. So to fix it you need struct uc_struct *uc = cpu->uc;. Have you even tried to compile this before you opened the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants