On 11/15/22 01:46, Eric Blake wrote:
The spec was silent on how many extents a server could reply with. However, both qemu and nbdkit (the two server implementations known to have implemented the NBD_CMD_BLOCK_STATUS extension) implement a hard cap, and will truncate the amount of extents in a reply to avoid sending a client a reply so large that the client would treat it as a denial of service attack. Clients currently have no way during negotiation to request such a limit of the server, so it is easier to just document this as a restriction on viable server implementations than to add yet another round of handshaking. Also, mentioning amplification effects is worthwhile.
When qemu first implemented NBD_CMD_BLOCK_STATUS for the base:allocation context (qemu commit e7b1948d51, Mar 2018), it behaved as if NBD_CMD_FLAG_REQ_ONE were always passed by the client, and never responded with more than one extent. Later, when adding its qemu:dirty-bitmap:XYZ context extension (qemu commit 3d068aff16, Jun 2018), it added a cap to 128k extents (1M+4 bytes), and that cap was applied to base:allocation once qemu started sending multiple extents for that context as well (qemu commit fb7afc797e, Jul 2018). Qemu extents are never smaller than 512 bytes (other than an exception at the end of a file whose size is not aligned to 512), but even so, a request for just under 4G of block status could produce 8M extents, resulting in a reply of 64M if it were not capped smaller.
When nbdkit first implemented NBD_CMD_BLOCK_STATUS (nbdkit 4ca66f70a5, Mar 2019), it did not impose any restriction on the number of extents in the reply chunk. But because it allows extents as small as one byte, it is easy to write a server that can amplify a client's request of status over 1M of the image into a reply over 8M in size, and it was very easy to demonstrate that a hard cap was needed to avoid crashing clients or otherwise killing the connection (a bad server impacting the client negatively). So nbdkit enforced a bound of 1M extents (8M+4 bytes, nbdkit commit 6e0dc839ea, Jun 2019). [Unrelated to this patch, but worth noting for history: nbdkit's situation also has to deal with the fact that it is designed for plugin server implementations; and not capping the number of extents in a reply also posed a problem to nbdkit as the server, where a plugin could exhaust memory and kill the server, unrelated to any size constraints enforced by a client.]
Since the limit chosen by these two implementations is different, and since nbdkit has versions that were not limited, add this as a SHOULD NOT instead of MUST NOT constraint on servers implementing block status. It does not matter that qemu picked a smaller limit that it truncates to, since we have already documented that the server may truncate for other reasons (such as it being inefficient to collect that many extents in the first place). But documenting the limit now becomes even more important in the face of a future addition of 64-bit requests, where a client's request is no longer bounded to 4G and could thereby produce even more than 8M extents for the corner case when every 512 bytes is a new extent, if it were not for this recommendation.
s-o-b line missed. Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> -- Best regards, Vladimir