Skip to content

sys: ring_buffer: fix possible ring_buf_put_claim() wrong size #88961

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

Merged

Conversation

butok
Copy link
Collaborator

@butok butok commented Apr 23, 2025

  • The issue is caused by the MIN() macro, which expands to (a)<(b)?(a):(b), where ring_buf_space_get()/ring_buf_size_get()
    is used as 'b' and is evaluated twice. The issue occurs when the (a)<(b) condition evaluates such that (b) is selected,
    but the value of (b) changes between evaluations, resulting in a possibly larger value than (a).
  • Fixes the potential incorrect behavior by storing the result of ring_buf_space_get()/ring_buf_size_get() in a variable before using it in the MIN macro.

@github-actions github-actions bot added the area: Base OS Base OS Library (lib/os) label Apr 23, 2025
@maass-hamburg maass-hamburg added the bug The issue is a bug, or the PR is fixing a bug label Apr 23, 2025
Copy link
Collaborator

@maass-hamburg maass-hamburg left a comment

Choose a reason for hiding this comment

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

should this also be done at ring_buf_get_claim() ?

- The issue is caused by the MIN() macro, which expands to
  (a)<(b)?(a):(b), where ring_buf_space_get()/ring_buf_size_get()
  is used as 'b' and is evaluated twice. The issue occurs when
  the (a)<(b) condition evaluates such that (b) is selected,
  but the value of (b) changes between evaluations, resulting
  in a possibly larger value than (a).
- Fixes the potential incorrect behavior by storing the result
  of ring_buf_space_get()/ring_buf_size_get() in a variable
  before using it in the MIN macro.

Signed-off-by: Andrej Butok <[email protected]>
@butok butok force-pushed the fix_ring_buf_put_claim_space branch from b45f96f to 5d05369 Compare April 23, 2025 12:34
@butok
Copy link
Collaborator Author

butok commented Apr 23, 2025

should this also be done at ring_buf_get_claim() ?

The requested ring_buf_get_claim() fix has been added.

@nashif nashif merged commit 0b3e9ab into zephyrproject-rtos:main Apr 26, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Base OS Base OS Library (lib/os) bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants