Skip to contentSkip to conversation

hwrng: virtio - fix copy_data() handling of partial reads

PATCH New Xiang Lei 2 participants 2 messages Nov 5

Subsystems (5)
Stable branch Crypto API virtio core Hardware random number generator core +1 more subsystems
lore
Share & tools
lore

Xiang Lei

author
#0 lore raw

PATCH 1/1 hwrng: virtio - fix copy_data() handling of partial reads

vi->data_avail is decremented on each read, so it holds the remaining bytes rather than the total length. Subtracting vi->data_idx from it again in commit e3046eeada29 ("hwrng: virtio: clamp device-reported used.len at copy_data()") undercounts the bytes left after a partial read. This can leave vi->data_avail nonzero without queuing a new request, causing virtio_read() to hang on have_data.

Clamp vi->data_avail to sizeof(vi->data) - vi->data_idx instead.

Fixes e3046eeada29 ("hwrng: virtio: clamp device-reported used.len at copy_data()")
Tested-by Wei Liu <[email protected]>
Cc [email protected]
Signed-off-by Xiang Lei <[email protected]>
---
 drivers/char/hw_random/virtio-rng.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/char/hw_random/virtio-rng.c +4 -4
index f9982217247d..a3392f7e2d8c 100644
@@ -70,7 +70,7 @@ static void request_entropy(struct virtrng_info *vi)
 static unsigned int copy_data(struct virtrng_info *vi, void *buf,
 			      unsigned int size)
 {
-	unsigned int idx, avail;
+	unsigned int idx;
 
 	/*
 	 * vi->data_avail was set from the device-reported used.len and

@@ -81,13 +81,13 @@ static unsigned int copy_data(struct virtrng_info *vi, void *buf,
 	 * steered into adjacent slab memory, including under
 	 * speculation.
 	 */
-	avail = min(vi->data_avail, sizeof(vi->data));
-	if (vi->data_idx >= avail) {
+	if (vi->data_idx >= sizeof(vi->data)) {
 		vi->data_avail = 0;
 		request_entropy(vi);
 		return 0;
 	}
-	size = min(size, avail - vi->data_idx);
+	vi->data_avail = min(vi->data_avail, sizeof(vi->data) - vi->data_idx);
+	size = min(size, vi->data_avail);
 	idx = array_index_nospec(vi->data_idx, sizeof(vi->data));
 	memcpy(buf, vi->data + idx, size);
 	vi->data_idx += size;
Signature · 1 line
2.53.0
AI review
#1 lore raw
Sashiko has reviewed this patch and found no issues.
Quoted 7 lines

vi->data_avail is decremented on each read, so it holds the remaining bytes rather than the total length. Subtracting vi->data_idx from it again in commit e3046eeada29 ("hwrng: virtio: clamp device-reported used.len at copy_data()") undercounts the bytes left after a partial read. This can leave vi->data_avail nonzero without queuing a new request, causing virtio_read() to hang on have_data.

Quoted 6 lines

Clamp vi->data_avail to sizeof(vi->data) - vi->data_idx instead.

Fixes e3046eeada29 ("hwrng: virtio: clamp device-reported used.len at copy_data()")
Tested-by Wei Liu <[email protected]>
Cc [email protected]
Signed-off-by Xiang Lei <[email protected]>

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1