Skip to content

Fix NULL deref segfault in bhyve's usb_mouse.c #1728

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jbendtsen
Copy link

@jbendtsen jbendtsen commented Jun 19, 2025

Bugzilla link:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282237

Some of the cases inside umouse_request() (usr.sbin/bhyve/usb_mouse.c) use the data component of an event, while only partially checking if it's NULL.

For example:

	case UREQ(UR_GET_STATUS, UT_READ_INTERFACE):
	case UREQ(UR_GET_STATUS, UT_READ_ENDPOINT):
		DPRINTF(("umouse: (UR_GET_STATUS, UT_READ_INTERFACE)"));
		if (data != NULL && len > 1) {
			USETW(udata, 0);
			data->blen = len - 2;
			data->bdone += 2;
		}
		eshort = data->blen > 0;
		break;

As you can see, 'data' has a NULL check, but then 'data' is immediately deferenced anyway after the check regardless of if it's NULL or not.

There are actually four occurrences of this same bug, each in a different case in this switch block.

Cheers,
Jack Bendtsen

Signed-off-by: Jack Bendtsen jackdbendtsen@gmail.com

Bugzilla link:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282237

> Some of the cases inside umouse_request() (usr.sbin/bhyve/usb_mouse.c) use the data component of an event, while only partially checking if it's NULL.
> 
> For example:
> ```
	case UREQ(UR_GET_STATUS, UT_READ_INTERFACE):
	case UREQ(UR_GET_STATUS, UT_READ_ENDPOINT):
		DPRINTF(("umouse: (UR_GET_STATUS, UT_READ_INTERFACE)"));
		if (data != NULL && len > 1) {
			USETW(udata, 0);
			data->blen = len - 2;
			data->bdone += 2;
		}
		eshort = data->blen > 0;
		break;
> ```
> As you can see, 'data' has a NULL check, but then 'data' is immediately deferenced anyway after the check regardless of if it's NULL or not.
> 
> There are actually four occurrences of this same bug, each in a different case in this switch block.
> 
> Cheers,
> Jack Bendtsen
@jbendtsen jbendtsen requested a review from bsdjhb as a code owner June 19, 2025 07:41
Copy link

github-actions bot commented Jun 19, 2025

Thank you for taking the time to contribute to FreeBSD!
There is an issue that needs to be fixed:

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.

1 participant