servo / webrender

A GPU-based renderer for the web
https://doc.servo.org/webrender/
Mozilla Public License 2.0
3.08k stars 269 forks source link

Unaligned pointer passed to `slice::from_raw_parts_mut` #4850

Open shinmao opened 3 months ago

shinmao commented 3 months ago

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. https://github.com/servo/webrender/blob/b6b2f65abb387d3214bad2041f0373718e76410b/wr_glyph_rasterizer/src/platform/unix/font.rs#L353-L357

https://github.com/servo/webrender/blob/b6b2f65abb387d3214bad2041f0373718e76410b/wr_glyph_rasterizer/src/platform/unix/font.rs#L361-L368

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 https://github.com/servo/webrender/blob/b6b2f65abb387d3214bad2041f0373718e76410b/wr_glyph_rasterizer/src/platform/unix/font.rs#L1062-L1068 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 commented 3 months ago

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!