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 invalid pointer dereference banned_cpumask_from_ui #141

Merged
merged 1 commit into from Nov 8, 2019

Conversation

@dublio
Copy link

dublio commented Nov 8, 2019

The memory of cpu_ban_string was release in sock_handle function,
so the banned_cpumask_from_ui will dereference an invalid memory.

Fix this issue by delay release memory.

Reproduce:
echo "settings cpus 0-3" | nc -U find /var/run/irqbalance/ -name *sock

Signed-off-by: Weiping Zhang zhangweiping@didiglobal.com

@nhorman

This comment has been minimized.

Copy link
Contributor

nhorman commented Nov 8, 2019

yeah, this looks like a legitimate bug, but thats alot of change to fix the problem. Why not just allocate cpu_ban_string in sock_handle, like we currently are, and have setup_banned_cpu check that pointer instead, do the grooming on the string (ie:
banned_cpumask_from_ui = strtok(cpu_ban_string, " ");
if (!strncmp(banned_cpumask_from_ui, "NULL", strlen("NULL"))) {
banned_cpumask_from_ui = NULL;
}
In setup_banned_cpus, and free it unilaterally at the end? That would just move 4 lines of code around and solve the problem just as well

@dublio

This comment has been minimized.

Copy link
Author

dublio commented Nov 8, 2019

yeah, this looks like a legitimate bug, but thats alot of change to fix the problem. Why not just allocate cpu_ban_string in sock_handle, like we currently are, and have setup_banned_cpu check that pointer instead, do the grooming on the string (ie:
banned_cpumask_from_ui = strtok(cpu_ban_string, " ");
if (!strncmp(banned_cpumask_from_ui, "NULL", strlen("NULL"))) {
banned_cpumask_from_ui = NULL;
}
In setup_banned_cpus, and free it unilaterally at the end? That would just move 4 lines of code around and solve the problem just as well

Good idea ^_^

The memory of cpu_ban_string was release in sock_handle function,
so the banned_cpumask_from_ui will dereference an invalid memory.

Fix this issue by delay release memory.

Reproduce:
echo "settings cpus 0-3" | nc -U `find /var/run/irqbalance/ -name *sock`

Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com>
@dublio dublio force-pushed the dublio:master branch from 1fc897e to 6c350eb Nov 8, 2019
@dublio

This comment has been minimized.

Copy link
Author

dublio commented Nov 8, 2019

@nhorman Hi, I have update patch with a bit more check before allocate memory for cpu_ban_string to avoid memory leak if prior one has not been released.

@nhorman

This comment has been minimized.

Copy link
Contributor

nhorman commented Nov 8, 2019

This looks better, thanks

@nhorman nhorman merged commit 3cbccb9 into Irqbalance:master Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.