Skip to content

Commit e6f8eed

Browse files
committed
murmur2: fix undefined behaviour
The Buffer struct held a MaybeUninit<[u8; 4]> which was written to byte-by-byte via assume_init_mut. The problem is that assume_init_mut requires the entire value is fully initialized. This fix changes Buffer.bytes to [MaybeUninit<u8>; 4], where each byte is its own MaybeUninit. Writes go through MaybeUninit::write() on individual elements which has no initialization requirement.
1 parent 7272d50 commit e6f8eed

2 files changed

Lines changed: 28 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](http://keepachangelog.com/)
66
and this project adheres to [Semantic Versioning](http://semver.org/).
77

8+
## [Unreleased]
9+
10+
### Fixed
11+
12+
- Fixed undefined behaviour in `Murmur3Hasher`.
13+
814
## [v1.0.0] - 2025-05-27
915

1016
### Added

src/murmur3.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct State(u32);
3030
#[derive(Debug, Clone, Copy)]
3131
#[repr(align(4))]
3232
struct Buffer {
33-
bytes: MaybeUninit<[u8; 4]>,
33+
bytes: [MaybeUninit<u8>; 4],
3434
}
3535

3636
#[derive(Debug, Clone, Copy, PartialEq)]
@@ -75,11 +75,10 @@ impl Murmur3Hasher {
7575
// self.buf.bytes[start..start+len].copy_from(buf);
7676
for i in 0..len {
7777
unsafe {
78-
*self
79-
.buf
78+
self.buf
8079
.bytes
81-
.assume_init_mut()
82-
.get_unchecked_mut(start + i) = *buf.get_unchecked(i);
80+
.get_unchecked_mut(start + i)
81+
.write(*buf.get_unchecked(i));
8382
}
8483
}
8584
self.index = Index::from(start + len);
@@ -90,7 +89,7 @@ impl Default for Murmur3Hasher {
9089
fn default() -> Self {
9190
Self {
9291
buf: Buffer {
93-
bytes: MaybeUninit::uninit(),
92+
bytes: [MaybeUninit::uninit(); 4],
9493
},
9594
index: Index::_0,
9695
processed: 0,
@@ -106,24 +105,24 @@ impl crate::Hasher for Murmur3Hasher {
106105
Index::_3 => {
107106
let mut block = 0;
108107
unsafe {
109-
block ^= u32::from(self.buf.bytes.assume_init_ref()[2]) << 16;
110-
block ^= u32::from(self.buf.bytes.assume_init_ref()[1]) << 8;
111-
block ^= u32::from(self.buf.bytes.assume_init_ref()[0]);
108+
block ^= u32::from(self.buf.bytes[2].assume_init()) << 16;
109+
block ^= u32::from(self.buf.bytes[1].assume_init()) << 8;
110+
block ^= u32::from(self.buf.bytes[0].assume_init());
112111
}
113112
self.state.0 ^ pre_mix(block)
114113
}
115114
Index::_2 => {
116115
let mut block = 0;
117116
unsafe {
118-
block ^= u32::from(self.buf.bytes.assume_init_ref()[1]) << 8;
119-
block ^= u32::from(self.buf.bytes.assume_init_ref()[0]);
117+
block ^= u32::from(self.buf.bytes[1].assume_init()) << 8;
118+
block ^= u32::from(self.buf.bytes[0].assume_init());
120119
}
121120
self.state.0 ^ pre_mix(block)
122121
}
123122
Index::_1 => {
124123
let mut block = 0;
125124
unsafe {
126-
block ^= u32::from(self.buf.bytes.assume_init_ref()[0]);
125+
block ^= u32::from(self.buf.bytes[0].assume_init());
127126
}
128127
self.state.0 ^ pre_mix(block)
129128
}
@@ -167,17 +166,20 @@ impl core::hash::Hasher for Murmur3Hasher {
167166
// self.buf.bytes[index..].copy_from_slice(head);
168167
for i in 0..4 - index {
169168
unsafe {
170-
*self
171-
.buf
169+
self.buf
172170
.bytes
173-
.assume_init_mut()
174-
.get_unchecked_mut(index + i) = *head.get_unchecked(i);
171+
.get_unchecked_mut(index + i)
172+
.write(*head.get_unchecked(i));
175173
}
176174
}
177175

178176
self.index = Index::_0;
179177

180-
self.state.process_block(&self.buf.bytes);
178+
// SAFETY: the loop above just wrote bytes [index..4], and prior push() calls
179+
// wrote bytes [0..index], so all 4 bytes are initialized.
180+
// In the future this can be replaced with array_assume_init
181+
let block: &[u8; 4] = unsafe { core::mem::transmute(&self.buf.bytes) };
182+
self.state.process_block(block);
181183

182184
body
183185
} else {
@@ -189,7 +191,7 @@ impl core::hash::Hasher for Murmur3Hasher {
189191
for block in body.chunks(4) {
190192
if block.len() == 4 {
191193
self.state
192-
.process_block(unsafe { &*(block.as_ptr().cast()) });
194+
.process_block(unsafe { &*(block.as_ptr().cast::<[u8; 4]>()) });
193195
} else {
194196
// NOTE(unsafe) In this branch, `block.len() < 4`. For CASE 1 and CASE 2 above,
195197
// `self.index.usize()` will be 0 here, so `self.index.usize() + block.len() < 4`.
@@ -213,8 +215,8 @@ const R1: u32 = 15;
213215

214216
impl State {
215217
#[allow(clippy::trivially_copy_pass_by_ref)]
216-
fn process_block(&mut self, block: &MaybeUninit<[u8; 4]>) {
217-
self.0 ^= pre_mix(u32::from_le_bytes(unsafe { *block.assume_init_ref() }));
218+
fn process_block(&mut self, block: &[u8; 4]) {
219+
self.0 ^= pre_mix(u32::from_le_bytes(*block));
218220
self.0 = self.0.rotate_left(13);
219221
self.0 = 5u32.wrapping_mul(self.0).wrapping_add(0xe6546b64);
220222
}

0 commit comments

Comments
 (0)