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

libmetal: Buffer overflow in metal_sys_io_mem_map() #323

Closed
iuliana-prodan opened this issue Feb 5, 2025 · 2 comments · Fixed by #324
Closed

libmetal: Buffer overflow in metal_sys_io_mem_map() #323

iuliana-prodan opened this issue Feb 5, 2025 · 2 comments · Fixed by #324

Comments

@iuliana-prodan
Copy link
Contributor

In void metal_sys_io_mem_map(struct metal_io_region *io) ([1] and [2]) if the I/O region size is a multiple of 1<<page_shift will result in a buffer overflow here, in the for loop:

for (p = 0; p <= (io->size >> io->page_shift); p++) {
	metal_machine_io_mem_map(va, io->physmap[p],
				 psize, io->mem_flags);
	va += psize;
}

The solution is:

diff --git a/libmetal/lib/system/freertos/io.c b/libmetal/lib/system/freertos/io.c
index 319322c..dde8242 100644
--- a/libmetal/lib/system/freertos/io.c
+++ b/libmetal/lib/system/freertos/io.c
@@ -22,7 +22,7 @@ void metal_sys_io_mem_map(struct metal_io_region *io)
        if (psize) {
                if (psize >> io->page_shift)
                        psize = (size_t)1 << io->page_shift;
-               for (p = 0; p <= (io->size >> io->page_shift); p++) {
+               for (p = 0; p <= ((io->size -1) >> io->page_shift); p++) {
                        metal_machine_io_mem_map(va, io->physmap[p],
                                                 psize, io->mem_flags);
                        va += psize;
diff --git a/libmetal/lib/system/generic/io.c b/libmetal/lib/system/generic/io.c
index 966bfc5..ac36f09 100644
--- a/libmetal/lib/system/generic/io.c
+++ b/libmetal/lib/system/generic/io.c
@@ -22,7 +22,7 @@ void metal_sys_io_mem_map(struct metal_io_region *io)
        if (psize) {
                if (psize >> io->page_shift)
                        psize = (size_t)1 << io->page_shift;
-               for (p = 0; p <= (io->size >> io->page_shift); p++) {
+               for (p = 0; p <= ((io->size -1) >> io->page_shift); p++) {
                        metal_machine_io_mem_map(va, io->physmap[p],
                                                 psize, io->mem_flags);
                        va += psize;

[1] https://github.com/OpenAMP/libmetal/blob/main/lib/system/freertos/io.c#L14
[2] https://github.com/OpenAMP/libmetal/blob/main/lib/system/generic/io.c#L14

@arnopo
Copy link
Contributor

arnopo commented Feb 5, 2025

Seems to me correct, don't hesitate to directly send a PR to propose the solution

@iuliana-prodan
Copy link
Contributor Author

Seems to me correct, don't hesitate to directly send a PR to propose the solution

Fix sent for review: #324

@arnopo arnopo linked a pull request Feb 12, 2025 that will close this issue
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 a pull request may close this issue.

2 participants