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 buffer overread and overrun in ipc #89

Merged
merged 1 commit into from
Jan 31, 2022

Conversation

matteodelabre
Copy link
Collaborator

Calls to msgsnd and msgrcv had an incorrect msgsz argument that was
consistently 4 bytes too long (it should not include the size of the
mtype field, as per the man page). This resulted in a buffer overread in
the case of msgsnd and overrun in the case of msgrcv.

Other implementations of rm2fb clients may also need to be fixed
(libremarkable, vnsee, waved). With this PR, the messages sent by bogus
clients that include 4 extra garbage bytes will still be accepted, but
truncated to fit in the actual structure, so as to preserve
compatibility until those clients are fixed.

Tested on rM2 2.11.0.442 with the following client apps: vnsee,
appmarkable, harmony, calculator, koreader (made sure that those apps
start and can send updates to the server successfully).

Calls to msgsnd and msgrcv had an incorrect msgsz argument that was
consistently 4 bytes too long (it should not include the size of the
mtype field, as per the man page). This resulted in a buffer overread in
the case of msgsnd and overrun in the case of msgrcv.

Other implementations of rm2fb clients may also need to be fixed
(libremarkable, vnsee, waved). With this PR, the messages sent by bogus
clients that include 4 extra garbage bytes will still be accepted, but
truncated to fit in the actual structure, so as to preserve
compatibility until those clients are fixed.

Tested on rM2 2.11.0.442 with the following client apps: vnsee,
appmarkable, harmony, calculator, koreader (made sure that those apps
start and can send updates to the server successfully).
Copy link
Collaborator

@raisjn raisjn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@matteodelabre matteodelabre merged commit 10464b7 into master Jan 31, 2022
@matteodelabre matteodelabre deleted the ipc-fix-msgsnd-overflow branch January 31, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants