Read::read_buf mutable reference to ReadBuf

[nrc]

cc #78485

Currently, Read::read_buf takes a &mut ReadBuf. That means that a malicious Read impl could swap out the buffer for another whereas the intent is that an impl should only write into the buffer. If users rely on properties of the ReadBuf to hold for an underlying buffer, then they must check that the ReadBuf has not been changed, otherwise they risk writing code which is unsound (see ##93305 for an example. Here copy_to should make this check, but does not). Since this check is not obvious, this is a potential footgun.

This was previously discussed on Zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Read.3A.3Abuf_read.20signature

Note that this is not a soundness issue, however, it makes it easier than it ought to be to write unsound code.

Note also that this requires an actively malicious impl of Read to be harmful since the lifetime of the ReadBuf means that the impl must use a static (or leaked) ReadBuf and that is extremely unlikely to happen by accident. IMO, this is important because a malicious impl could cause similar errors in many other ways, such as calling assume_init on the ReadBuf without in fact initializing the bytes. That does require unsafe, but IMO is no easier to audit than swapping out the ReadBuf for another (since every legitimate use of ReadBuf requires unsafe code).

Anyway, the way to fix this is to have some kind of type struct ReadBufMut<'a>(&'a mut ReadBuf); and to use ReadBufMut rather than &mut ReadBuf in read_buf's signature. The interesting question is exactly how to convert between the two types ergonomically and whether we can design things in such a way that we can expose only one type rather than two.

[Darksonn]

The Tokio crate already uses a ReadBuf-based API in its IO traits, and my experience using the ReadBuf design there for more than a year is that this is important to fix. Pretty much everyone who writes something that can have this mistake does make the mistake until pointed out to them.

Regarding ergonomics, one way could be to have ReadBuf have an .as_mut() method that returns an ReadBufMut<'_>.

[beepster4096]

#93359 implements this

[tmiasko]

Resolved by #97015.