Skip to content

gh-141311: Avoid assertion in BytesIO readinto #141333

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Nov 12, 2025
Merged

gh-141311: Avoid assertion in BytesIO readinto #141333

merged 9 commits into from
Nov 12, 2025
+29 −3

Conversation

@cmaloney
Copy link
Contributor

@cmaloney cmaloney commented Nov 10, 2025 •
edited by bedevere-app bot
Loading

Account for when self->pos is equal to PY_SSIZE_T_MAX. The length of read was correctly set to zero but the asserts assumed self->pos couldn't reach PY_SSIZE_T_MAX. Return early to avoid edge cases.

@cmaloney
pythongh-141311: Avoid assertion in BytesIO readinto
db81df6 
Account for when self->pos is equal to PY_SSIZE_T_MAX. The length of
read was correctly set to zero but the asserts assumed
self->pos couldn't reach PY_SSIZE_T_MAX. Return early to avoid edge
cases.
@bedevere-app bedevere-app bot mentioned this pull request Nov 10, 2025
Open
serhiy-storchaka
serhiy-storchaka reviewed Nov 10, 2025
View reviewed changes
@cmaloney
Simplify by using sys.maxsize
2c84efe
@cmaloney
Reference io.BytesIO.readinto directly
d119904
@cmaloney
Resolve read_bytes_lock_held and audit other cases
d78cc84 
For `read_bytes_lock_held` the `size` parameter is always set to 0 by calling
code currently if self->pos > self->string_size. I added a range check for
self->pos as an extra safety as codepaths chagne but can remove if it's too much.

`write_bytes_lock_held`: `endpos` is a `size_t` so can hold
`2 * PY_SSIZE_T_MAX`. Value is bounds checked in `resize_buffer_lock_held`.

A number of cases use `scan_eol_lock_held` to move forward. That has code which
checks `self->pos >= self->string_size` and returns `0`. Callsites all seem to
handle that correctly.
@cmaloney
Copy link
Contributor Author

cmaloney commented Nov 11, 2025

I audited the codepaths which did self->pos + buffer.

For read_bytes_lock_held the size parameter is always set to 0 by calling code currently if self->pos > self->string_size. I added a range check for self->pos as an extra safety as codepaths change but can remove if it's too much.

write_bytes_lock_held: endpos is a size_t so can hold 2 * PY_SSIZE_T_MAX. Value is bounds checked in resize_buffer_lock_held.

A number of cases use scan_eol_lock_held to move forward. That has code which checks self->pos >= self->string_size and returns 0. Callers seem to handle that correctly (early exit or call read_bytes_lock_held)..

serhiy-storchaka
serhiy-storchaka reviewed Nov 11, 2025
View reviewed changes
serhiy-storchaka
serhiy-storchaka reviewed Nov 11, 2025
View reviewed changes
serhiy-storchaka
serhiy-storchaka approved these changes Nov 11, 2025
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@serhiy-storchaka serhiy-storchaka merged commit 7d54374 into python:main Nov 12, 2025
50 checks passed
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Nov 12, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Nov 12, 2025

Thanks @cmaloney for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Nov 12, 2025

Thanks @cmaloney for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Nov 12, 2025

Sorry, @cmaloney and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 7d54374f9c7d91e0ef90c4ad84baf10073cf1d8a 3.13

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 12, 2025
@cmaloney @miss-islington
pythongh-141311: Avoid assertion in BytesIO.readinto() (pythonGH-141333)
7151cfb 
Fix error in assertion which causes failure if pos is equal to PY_SSIZE_T_MAX.
Fix undefined behavior in read() and readinto() if pos is larger that the size
of the underlying buffer.
(cherry picked from commit 7d54374)

Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Nov 12, 2025

GH-141457 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Nov 12, 2025
@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Nov 12, 2025

@cmaloney, could you please create a backport to 3.13 manually?

serhiy-storchaka pushed a commit that referenced this pull request Nov 12, 2025
@miss-islington @cmaloney
[3.14] gh-141311: Avoid assertion in BytesIO.readinto() (GH-141333) (G…
75b5157 
…H-141457)

Fix error in assertion which causes failure if pos is equal to PY_SSIZE_T_MAX.
Fix undefined behavior in read() and readinto() if pos is larger that the size
of the underlying buffer.
(cherry picked from commit 7d54374)

Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

@serhiy-storchaka serhiy-storchaka

Labels

needs backport to 3.13 bugs and security fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cmaloney @serhiy-storchaka