Skip to contentSkip to conversation

hwrng: virtio - use min() in copy_data()

PATCH In review Thorsten Blum 5 participants 7 messages Sep 26 – Oct 2

Subsystems (4)
Crypto API virtio core Hardware random number generator core virtio rng driver
lore
Share & tools
lore

Thorsten Blum

author
#0 lore raw

PATCH 1/2 hwrng: virtio - use min() in copy_data()

Use the simpler min() macro since all values are unsigned and compatible. The subtraction avail - vi->data_idx is also guaranteed to be positive by the preceding check.

Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/char/hw_random/virtio-rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/char/hw_random/virtio-rng.c +2 -2
index 5e83ffa105e4..5b1b23b5596a 100644
@@ -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_t(unsigned int, vi->data_avail, sizeof(vi->data));
+	avail = min(vi->data_avail, sizeof(vi->data));
 	if (vi->data_idx >= avail) {
 		vi->data_avail = 0;
 		request_entropy(vi);
 		return 0;
 	}
-	size = min_t(unsigned int, size, avail - vi->data_idx);
+	size = min(size, avail - vi->data_idx);
 	idx = array_index_nospec(vi->data_idx, sizeof(vi->data));
 	memcpy(buf, vi->data + idx, size);
 	vi->data_idx += size;
author
#1 lore raw

PATCH 2/2 hwrng: virtio - use snprintf() in probe_common()

While the current code works correctly, replace the unbounded sprintf() with the safer snprintf() to follow secure coding best practices.

Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/char/hw_random/virtio-rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/hw_random/virtio-rng.c +1 -1
index 5b1b23b5596a..f9982217247d 100644
@@ -162,7 +162,7 @@ static int probe_common(struct virtio_device *vdev)
 		err = index;
 		goto err_ida;
 	}
-	sprintf(vi->name, "virtio_rng.%d", index);
+	snprintf(vi->name, sizeof(vi->name), "virtio_rng.%d", vi->index);
 	init_completion(&vi->have_data);
 
 	vi->hwrng = (struct hwrng) {
AI review
#2 lore raw

Re: [PATCH 2/2] hwrng: virtio - use snprintf() in probe_common()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 4 lines

While the current code works correctly, replace the unbounded sprintf() with the safer snprintf() to follow secure coding best practices.

Signed-off-by Thorsten Blum <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Reviewed-by on v1 2/2
#3 lore raw

Re: [PATCH 2/2] hwrng: virtio - use snprintf() in probe_common()

In reply to Thorsten Blum
Quoted 13 lines · Thorsten Blum

While the current code works correctly, replace the unbounded sprintf() with the safer snprintf() to follow secure coding best practices.

Signed-off-by Thorsten Blum <[email protected]>
---
  drivers/char/hw_random/virtio-rng.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/hw_random/virtio-rng.c +0 -0
index 5b1b23b5596a..f9982217247d 100644

drivers/char/hw_random/virtio-rng.c static int probe_common(struct virtio_device *vdev) · line 168

Quoted 8 lines
drivers/char/hw_random/virtio-rng.c +1 -1
@@ -162,7 +162,7 @@ static int probe_common(struct virtio_device *vdev)
  		err = index;
  		goto err_ida;
  	}
-	sprintf(vi->name, "virtio_rng.%d", index);
+	snprintf(vi->name, sizeof(vi->name), "virtio_rng.%d", vi->index);
  	init_completion(&vi->have_data);
 
  	vi->hwrng = (struct hwrng) {
Reviewed-by Thomas Huth <[email protected]>
maintainer Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] hwrng: virtio - use snprintf() in probe_common()

In reply to Thorsten Blum
Quoted 14 lines · Thorsten Blum

While the current code works correctly, replace the unbounded sprintf() with the safer snprintf() to follow secure coding best practices.

Signed-off-by Thorsten Blum <[email protected]>
---
  drivers/char/hw_random/virtio-rng.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/hw_random/virtio-rng.c +0 -0
index 5b1b23b5596a..f9982217247d 100644

drivers/char/hw_random/virtio-rng.c static int probe_common(struct virtio_device *vdev) · line 168

Quoted 8 lines
drivers/char/hw_random/virtio-rng.c +1 -1
@@ -162,7 +162,7 @@ static int probe_common(struct virtio_device *vdev)
  		err = index;
  		goto err_ida;
  	}
-	sprintf(vi->name, "virtio_rng.%d", index);
+	snprintf(vi->name, sizeof(vi->name), "virtio_rng.%d", vi->index);
  	init_completion(&vi->have_data);
 
  	vi->hwrng = (struct hwrng) {
Reviewed-by Laurent Vivier <[email protected]>
AI review
#5 lore raw
In reply to Thorsten Blum
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 5 lines

Use the simpler min() macro since all values are unsigned and compatible. The subtraction avail - vi->data_idx is also guaranteed to be positive by the preceding check.

Signed-off-by Thorsten Blum <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer
#6 lore raw
In reply to Thorsten Blum
Quoted 8 lines · Thorsten Blum

Use the simpler min() macro since all values are unsigned and compatible. The subtraction avail - vi->data_idx is also guaranteed to be positive by the preceding check.

Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/char/hw_random/virtio-rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

All applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt