Unaligned pointer passed to slice::from_raw_parts_mut

[shinmao]

Unsoundness

Hi, we considered that flip_bitmap_x and flip_bitmap_y break the alignment requirements of unsafe function slice::from_raw_parts_mut with unaligned raw pointer.

webrender/wr_glyph_rasterizer/src/platform/unix/font.rs

Lines 353 to 357 in b6b2f65

	fn flip_bitmap_x(bitmap: &mut [u8], width: usize, height: usize) {
	assert!(bitmap.len() == width * height * 4);
	let pixels = unsafe { slice::from_raw_parts_mut(bitmap.as_mut_ptr() as *mut u32, width * height) };
	for row in pixels.chunks_mut(width) {
	row.reverse();

webrender/wr_glyph_rasterizer/src/platform/unix/font.rs

Lines 361 to 368 in b6b2f65

	fn flip_bitmap_y(bitmap: &mut [u8], width: usize, height: usize) {
	assert!(bitmap.len() == width * height * 4);
	let pixels = unsafe { slice::from_raw_parts_mut(bitmap.as_mut_ptr() as *mut u32, width * height) };
	for y in 0 .. height / 2 {
	let low_row = y * width;
	let high_row = (height - 1 - y) * width;
	for x in 0 .. width {
	pixels.swap(low_row + x, high_row + x);

Since both of the functions are private, we also checked whether the callers of functions passed the type aligned to 4 bytes actually in the library. We found that in the function rasterize_glyph

webrender/wr_glyph_rasterizer/src/platform/unix/font.rs

Lines 1062 to 1068 in b6b2f65

	if font.flags.contains(FontInstanceFlags::FLIP_X) {
	flip_bitmap_x(&mut final_buffer, actual_width, actual_height);
	left = -(left + actual_width as i32);
	}
	if font.flags.contains(FontInstanceFlags::FLIP_Y) {
	flip_bitmap_y(&mut final_buffer, actual_width, actual_height);
	top = -(top - actual_height as i32);

final_buffer is created from u8 slice in line 948, which is aligned to 1 byte. Therefore, we are sure that final_buffer is cast to 4 bytes and creates an unaligned pointer.

When the slice is created from unaligned raw pointer, the undefined behavior could lead to inconsistent results in different systems and architectures, which will change the pixels values here. Considering use ptr::copy_non_overlapping to create a new type aligned to 4 bytes before passing into from_raw_parts would be more safe.

[mrobinson]

Hi! Thanks for reporting this issue. Do you mind reporting it at https://bugzilla.mozilla.org. This repository is just a downstream mirror of the WebRender with some patches applied for Servo. Almost all work on WebRender is happening upstream in the Gecko repository. Thanks!