Skip to content

Allow writes of length 0 to a full buffer #15592

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
merged 1 commit into from
Jul 12, 2014

Conversation

arjantop
Copy link
Contributor

First condition is not needed and just prevents 0 length writes

Fixes #15583

@alexcrichton
Copy link
Member

Could you add a test for this as well?

@arjantop
Copy link
Contributor Author

Yeah just remembered, working on that

@bluss
Copy link
Member

bluss commented Jul 10, 2014

You'd write buf.len() > max_size - self.pos to be safe against overflow. I'm not sure if the surrounding code is written with that in mind, or if it matters, though.

(re-post comment from changed diff to here)

@arjantop
Copy link
Contributor Author

Added test and changed condition to @blake2-ppc suggestion

@arjantop
Copy link
Contributor Author

Looks like there ia a problem with a @blake2-ppc suggestion because seeking beyond end of buffer is allowed and all the lengths are unsigned.

@bluss
Copy link
Member

bluss commented Jul 11, 2014

True, I'm sorry for that; the conditional needs an invariant that Seek doesn't keep in place. The fact remains that the original condition is an "overflow trigger", a pattern that usually has an overflow problem.

In the past I fixed a lot of overflow problems with vectors -- Rust code would write overflowing calculations and head directly into unsafe code with those invalid values (allowing crashes or arbitrary memory clobbering). Since there's always a risk that later changes add unsafe sections to some parts of the implementation of for example BufWriter, it remains important to think about arithmetic wraparound / overflow.

@arjantop
Copy link
Contributor Author

That was just a statement after finding a problem until I fix it, not blaming you.

@arjantop
Copy link
Contributor Author

That build error just travis issues? @alexcrichton

@huonw
Copy link
Member

huonw commented Jul 12, 2014

@arjantop probably, I just restarted it. (Also, btw, bors the integration bot ignores travis results, i.e. it will do it's thing even if the travis build fails. e.g. this PR is still in the approved section of the queue, and will probably be tested in a few hours.)

bors added a commit that referenced this pull request Jul 12, 2014
First condition is not needed and just prevents 0 length writes

Fixes #15583
@bors bors closed this Jul 12, 2014
@bors bors merged commit 30f07e9 into rust-lang:master Jul 12, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
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.

Writing 0 bytes to a full buffer results in an error
6 participants