Bad pun. But while debugging the issue Bjoern raised [1] I've noticed these problems. https://www.redhat.com/archives/libvir-list/2018-July/msg02101.html Michal Prívozník (2): util: Don't overflow in virRandomBits viriscsi: Request more random bits for interface name src/util/viriscsi.c | 2 +- src/util/virrandom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wednesday, 1 August 2018 13:44:31 CEST Michal Privoznik wrote: > Bad pun. But while debugging the issue Bjoern raised [1] I've noticed > these problems. > > https://www.redhat.com/archives/libvir-list/2018-July/msg02101.html > > Michal Prívozník (2): > util: Don't overflow in virRandomBits > viriscsi: Request more random bits for interface name > > src/util/viriscsi.c | 2 +- > src/util/virrandom.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Since we worked both on this, then: Reviewed-by: Pino Toscano <ptoscano@redhat.com> -- Pino Toscano-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
And here's the fix for the viriscsitest on big-endian machine like
Daniel suggested.
Bjoern
--
IBM Systems
Linux on Z & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
------------------------------------------------------------------------
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001
From: Bjoern Walk <bwalk@linux.ibm.com>
Date: Wed, 1 Aug 2018 14:48:47 +0200
Subject: [PATCH] util: virrandom: make virRandomBits endian-safe
Make the generation of random bits in virRandomBits independent of the
endianness of the running architecture.
This also solves problems with the mocked random byte generation on
big-endian machines.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
---
src/util/virrandom.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 01cc82a0..a58ee97a 100644
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -34,6 +34,7 @@
# include <gnutls/crypto.h>
#endif
+#include "virendian.h"
#include "virrandom.h"
#include "virthread.h"
#include "count-one-bits.h"
@@ -60,16 +61,15 @@ VIR_LOG_INIT("util.random");
*/
uint64_t virRandomBits(int nbits)
{
- uint64_t ret = 0;
+ uint8_t ret[8];
- if (virRandomBytes((unsigned char *) &ret, sizeof(ret)) < 0) {
+ if (virRandomBytes(ret, sizeof(ret)) < 0) {
/* You're already hosed, so this particular non-random value
* isn't any worse. */
return 0;
}
- ret &= (1ULL << nbits) - 1;
- return ret;
+ return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1);
}
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Aug 01, 2018 at 02:57:07PM +0200, Bjoern Walk wrote:
> And here's the fix for the viriscsitest on big-endian machine like
> Daniel suggested.
>
> Bjoern
>
> --
> IBM Systems
> Linux on Z & Virtualization Development
> ------------------------------------------------------------------------
> IBM Deutschland Research & Development GmbH
> Schönaicher Str. 220, 71032 Böblingen
> Phone: +49 7031 16 1819
> ------------------------------------------------------------------------
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
> From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001
> From: Bjoern Walk <bwalk@linux.ibm.com>
> Date: Wed, 1 Aug 2018 14:48:47 +0200
> Subject: [PATCH] util: virrandom: make virRandomBits endian-safe
>
> Make the generation of random bits in virRandomBits independent of the
> endianness of the running architecture.
>
> This also solves problems with the mocked random byte generation on
> big-endian machines.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
> ---
> src/util/virrandom.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/util/virrandom.c b/src/util/virrandom.c
> index 01cc82a0..a58ee97a 100644
> --- a/src/util/virrandom.c
> +++ b/src/util/virrandom.c
> @@ -34,6 +34,7 @@
> # include <gnutls/crypto.h>
> #endif
>
> +#include "virendian.h"
> #include "virrandom.h"
> #include "virthread.h"
> #include "count-one-bits.h"
> @@ -60,16 +61,15 @@ VIR_LOG_INIT("util.random");
> */
> uint64_t virRandomBits(int nbits)
> {
> - uint64_t ret = 0;
> + uint8_t ret[8];
>
> - if (virRandomBytes((unsigned char *) &ret, sizeof(ret)) < 0) {
> + if (virRandomBytes(ret, sizeof(ret)) < 0) {
> /* You're already hosed, so this particular non-random value
> * isn't any worse. */
> return 0;
> }
>
> - ret &= (1ULL << nbits) - 1;
> - return ret;
> + return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1);
> }
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hi Bjoern, On Wednesday, 1 August 2018 14:57:07 CEST Bjoern Walk wrote: > And here's the fix for the viriscsitest on big-endian machine like > Daniel suggested. I sent an alternative fix, only for viriscsitest, while you sent this patch. IMHO virRandomBits() ought to not deal with endianness, since all it sees are random values, without any meaning. The problem was specific to the test, because it assumed a certain order of the data returned by the mocked virRandomBytes(). -- Pino Toscano-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Pino Toscano <ptoscano@redhat.com> [2018-08-01, 03:21PM +0200]: > Hi Bjoern, > > On Wednesday, 1 August 2018 14:57:07 CEST Bjoern Walk wrote: > > And here's the fix for the viriscsitest on big-endian machine like > > Daniel suggested. > > I sent an alternative fix, only for viriscsitest, while you sent this > patch. > > IMHO virRandomBits() ought to not deal with endianness, since all it > sees are random values, without any meaning. The problem was specific > to the test, because it assumed a certain order of the data returned by > the mocked virRandomBytes(). Yes, I agree. That's why I hesitated sending this fix. But this was also the solution that Daniel suggested and maybe there is value in having the random bit generation endian-safe as well (although I don't see one currently...). I actually don't care how we fix it. I will test your patch and report back. > > -- > Pino Toscano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/01/2018 07:57 AM, Bjoern Walk wrote: > And here's the fix for the viriscsitest on big-endian machine like > Daniel suggested. > From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001 > From: Bjoern Walk<bwalk@linux.ibm.com> > Date: Wed, 1 Aug 2018 14:48:47 +0200 > Subject: [PATCH] util: virrandom: make virRandomBits endian-safe > > Make the generation of random bits in virRandomBits independent of the > endianness of the running architecture. > > This also solves problems with the mocked random byte generation on > big-endian machines. > > Suggested-by: Daniel P. Berrangé<berrange@redhat.com> > Signed-off-by: Bjoern Walk<bwalk@linux.ibm.com> > --- > src/util/virrandom.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > > - ret &= (1ULL << nbits) - 1; > - return ret; > + return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1); Needs to be rebased on top of the fix that (1ULL << 64) is not defined. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Eric Blake <eblake@redhat.com> [2018-08-01, 09:51AM -0500]: > On 08/01/2018 07:57 AM, Bjoern Walk wrote: > > And here's the fix for the viriscsitest on big-endian machine like > > Daniel suggested. > > > From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001 > > From: Bjoern Walk<bwalk@linux.ibm.com> > > Date: Wed, 1 Aug 2018 14:48:47 +0200 > > Subject: [PATCH] util: virrandom: make virRandomBits endian-safe > > > > Make the generation of random bits in virRandomBits independent of the > > endianness of the running architecture. > > > > This also solves problems with the mocked random byte generation on > > big-endian machines. > > > > Suggested-by: Daniel P. Berrangé<berrange@redhat.com> > > Signed-off-by: Bjoern Walk<bwalk@linux.ibm.com> > > --- > > src/util/virrandom.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > - ret &= (1ULL << nbits) - 1; > > - return ret; > > + return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1); > > Needs to be rebased on top of the fix that (1ULL << 64) is not defined. Yes, agreed. But now the original patch has been pushed... -- IBM Systems Linux on Z & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 ------------------------------------------------------------------------ Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.