diff options
Diffstat (limited to 'vendor/image/docs/2019-04-23-memory-unsafety.md')
-rw-r--r-- | vendor/image/docs/2019-04-23-memory-unsafety.md | 54 |
1 files changed, 54 insertions, 0 deletions
diff --git a/vendor/image/docs/2019-04-23-memory-unsafety.md b/vendor/image/docs/2019-04-23-memory-unsafety.md new file mode 100644 index 0000000..3989eb2 --- /dev/null +++ b/vendor/image/docs/2019-04-23-memory-unsafety.md @@ -0,0 +1,54 @@ +# Advisory about potential memory unsafety issues + +[While reviewing][i885] some `unsafe Vec::from_raw_parts` operations within the +library, trying to justify their existence with stronger reasoning, we noticed +that they instead did not meet the required conditions set by the standard +library. This unsoundness was quickly removed, but we noted that the same +unjustified reasoning had been applied by a dependency introduced in `0.21`. + +For efficiency reasons, we had tried to reuse the allocations made by decoders +for the buffer of the final image. However, that process is error prone. Most +image decoding algorithms change the representation type of color samples to +some degree. Notably, the output pixel type may have a different size and +alignment than the type used in the temporary decoding buffer. In this specific +instance, the `ImageBuffer` of the output expects a linear arrangement of `u8` +samples while the implementation of the `hdr` decoder uses a pixel +representation of `Rgb<u8>`, which has three times the size. One of the +requirements of `Vec::from_raw_parts` reads: + +> ptr's T needs to have the same size and alignment as it was allocated with. + +This requirement is not present on slices `[T]`, as it is motivated by the +allocator interface. The validity invariant of a reference and slice only +requires the correct alignment here, which was considered in the design of +`Rgb<_>` by giving it a well-defined representation, `#[repr(C)]`. But +critically, this does not guarantee that we can reuse the existing allocation +through effectively transmuting a `Vec<_>`! + +The actual impact of this issue is, in real world implementations, limited to +allocators which handle allocations for types of size `1` and `3`/`4` +differently. To the best of my knowledge, this does not apply to `jemalloc` and +the `libc` allocator. However, we decided to proceed with caution here. + +## Lessons for the future + +New library dependencies will be under a stricter policy. Not only would they +need to be justified by functionality but also require at least some level of +reasoning how they solve that problem better than alternatives. Some appearance +of maintenance, or the existence of `#[deny(unsafe)]`, will help. We'll +additionally look into existing dependencies trying to identify similar issues +and minimizing the potential surface for implementation risks. + +## Sound and safe buffer reuse + +It seems that the `Vec` representation is entirely unfit for buffer reuse in +the style which an image library requires. In particular, using pixel types of +different sizes is likely common to handle either whole (encoded) pixels or +individual samples. Thus, we started a new sub-project to address this use +case, [image-canvas][image-canvas]. Contributions and review of its safety are +very welcome, we ask for the communities help here. The release of `v0.1` will +not occur until at least one such review has occurred. + + +[i885]: https://github.com/image-rs/image/pull/885 +[image-canvas]: https://github.com/image-rs/canvas |