aboutsummaryrefslogtreecommitdiff
path: root/vendor/image/docs
diff options
context:
space:
mode:
Diffstat (limited to 'vendor/image/docs')
-rw-r--r--vendor/image/docs/2019-04-23-memory-unsafety.md54
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