Misaligned pointer dereference in get_bitmap

[shinmao]

The source of unsoundness

Hi, we found that the safe function get_bitmap might include unsound implementation.

fontdue/src/platform/float/get_bitmap.rs

Lines 59 to 60 in 9e7bacf

	let pointer: &mut i32 = core::mem::transmute(output.get_unchecked_mut(i));
	*pointer = core::mem::transmute::<__m128i, [i32; 4]>(y)[0];

At line 59, the mutable pointer from output (aligned to 1 byte) is transmuted to &mut i32 (aligned to 4 bytes), and this created a misaligned pointer. Misaligned pointer dereference at line 60 can leads to undefined behavior.

[mooman219]

You're right that it looks like unaligned reads, but x86_64 implementations from AMD and Intel create memory allocations aligned to usually at least 16 byte boundaries, and we always read in 4 byte chunks, so we never actually end up with an unaligned read since we're reading 4 byte chunks from a 16 byte aligned allocation.

Probably undefined behavior as far as rust is concerned, I'd have to revisit the docs to see if they guarantee some sort of alignment on the initial allocation in writing. If not I can explicitly request the alignment on allocation which should be a no-op

[shinmao]

@mooman219 Yeah. I know what you mean. However, the allocated vec is not always aligned to >= 16 bytes as I know. Please see the link:
rust-lang/unsafe-code-guidelines#38 (comment)
It still depends on the alignment of elements, and it could be greater than it.

Here is the official unsafe code reference to talk about the layout of packed SIMD vector:
https://rust-lang.github.io/unsafe-code-guidelines/layout/packed-simd-vectors.html

[mooman219]

Fair enough, I couldn't actually get it to allocate unaligned after fiddling with compiler settings for a little bit, so it's not likely to be an issue, but it's definitely compiler implementation defined. That being said, unaligned reads are benign security wise on the targets this codepath is enabled for at least.

I'll get this resolved, thanks for the heads up!

[mooman219]

Published a new version and re-benched. The change is within noise. Thanks for the analysis!