Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

Various netlink-proto Encoder impl fixes #168

Merged
merged 3 commits into from
Jun 14, 2021

Conversation

stbuehler
Copy link
Contributor

Afaict the missing reserve before line 175 might actually cause panics when serializing messages that need more space than the current buffer capacity provides.

Hitting buf.remaining_mut() < msg_len is quite unlikely, but would probably end in OOM (and loop until it does).

fe8cb5e introduced this when to_bytes()
didn't return the size anymore.
- don't allocate in 2048 steps; we know the total size of additional
  space required (added in d1deb12)
- BytesMut can expand to usize::MAX now; so checking remaining_mut()
  only prevents a size overflow (can't allocate it anyway).
- BytesMut::resize actually changes the size of the buffer and reduces
  `remaining_mut`
- need to actually reserve enough space before taking
  `&mut buf.chunk_mut()[..msg_len];`
The only semantic difference: on unwind (panic in serialize) the buffer
length already is the new one (but the contents were initialized to 0 anyway).
@stbuehler
Copy link
Contributor Author

The missing reserve issue can be easily demonstrated by setting INITIAL_WRITER_CAPACITY = 0 in

const INITIAL_WRITER_CAPACITY: usize = 8 * 1024;
and trying to send a message bigger than 64 bytes (the default number of bytes BytesMut::chunk_mut() allocates if capacity is reached). Without manipulating INITIAL_WRITER_CAPACITY one might have to send a message > 8k to trigger it.

Copy link
Collaborator

@JohnTitor JohnTitor 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 to me. The commit messages helped a lot to understand the relations, thanks!

@cathay4t cathay4t merged commit 9d2a995 into little-dude:master Jun 14, 2021
@stbuehler stbuehler deleted the proto-encoder branch October 30, 2021 20:30
little-dude pushed a commit that referenced this pull request Nov 22, 2021
Various netlink-proto Encoder impl fixes
@little-dude little-dude mentioned this pull request Dec 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants