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

Buffer overflow #44

Closed
Peter-van-Tol opened this issue Sep 18, 2023 · 0 comments · Fixed by #45
Closed

Buffer overflow #44

Peter-van-Tol opened this issue Sep 18, 2023 · 0 comments · Fixed by #45

Comments

@Peter-van-Tol
Copy link
Owner

Peter-van-Tol commented Sep 18, 2023

Describe the bug
The communication to the card fails with the following message:

Unexpected read length: -1, expected 88
Unexpected read length: -1, expected 88
Unexpected read length: -1, expected 88
Unexpected read length: -1, expected 88
Unexpected read length: -1, expected 88

The card itself is still responsive, as the card is successfully reset at start up and shut down of LinuxCNC.

Cause
The buffer containing the addresses to be read is partly over-written. The magic byte is erased and therefore the FPGA does not recognize the packet as a read command.

Solution
The culprit in this essence is the clearing the read and write buffers:

    // Clear buffer (except for the header)
    memset(
        litexcnc->fpga->read_buffer + litexcnc->fpga->read_header_size, 
        0, 
        litexcnc->fpga->read_buffer_size
    ); 

This code will cause a write outside of the buffer, because the offset is not applied to the length.

Correct way is:

    // Clear buffer (except for the header)
    memset(
        litexcnc->fpga->read_buffer + litexcnc->fpga->read_header_size, 
        0, 
        litexcnc->fpga->read_buffer_size - litexcnc->fpga->read_header_size
    ); 
@Peter-van-Tol Peter-van-Tol linked a pull request Sep 18, 2023 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.

1 participant