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

Memory leak in a loop in scan_mem function #44

Open
eupharina opened this issue Sep 27, 2024 · 2 comments
Open

Memory leak in a loop in scan_mem function #44

eupharina opened this issue Sep 27, 2024 · 2 comments

Comments

@eupharina
Copy link

While reviewing static analysis reports on some CheriBSD ports, I came across this code:

		char *query_value;
		asprintf(&query_value, "(\"0x%lx\", \"0x%lx\", \"%s\", %d, %d, %d, %d)", 
				kivp->kve_start,
				kivp->kve_end,
				mmap_path,
				compart_id,
				kivp->kve_protection,
				kivp->kve_flags,
				kivp->kve_type);
	
		if (i == 0) {
                        // CSA warning below: Allocator sizeof operand mismatch
			insert_vm_query_values = (char*)malloc(sizeof(query_value)); 
			assert(insert_vm_query_values != NULL);


			insert_vm_query_values = strdup(query_value);
		} else {
			char *temp;
			asprintf(&temp, "%s,%s", 
				insert_vm_query_values,
				query_value);
			insert_vm_query_values = strdup(temp);
			free(temp);
		}
		free(query_value);

I don't have much context of what is going on here, but this looks like a memory leak in a loop to me: insert_vm_query_values is overwritten (both in i==0 and else branches) without freeing it's previous value. It's probably a good idea to rewrite this bit of code anyway with fewer string copies.

@psjm3
Copy link
Contributor

psjm3 commented Nov 13, 2024

Thank you! Yes, the code was written pretty much in experimental mode, there are a few places I will need to refactor to fix a few bugs and for optimisation. The reason I haven't done much tidying up and optimisation yet is because I am still implementing the basic functionality, and there is a good chance a lot of the existing code will be removed once we figured out how the code should be structured (e.g. I have plans to change the command line options and libraries into pluggable modules, etc.).

But this could be a trivial fix, will definitely address them.

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

No branches or pull requests

2 participants