src/util/virrandom.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
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>
---
This goes on top of Michal's fix: https://www.redhat.com/archives/libvir-list/2018-August/msg00080.html
src/util/virrandom.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 7915f653..26ff68f5 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"
@@ -61,13 +62,16 @@ VIR_LOG_INIT("util.random");
uint64_t virRandomBits(int nbits)
{
uint64_t ret = 0;
+ uint8_t val[8];
- if (virRandomBytes((unsigned char *) &ret, sizeof(ret)) < 0) {
+ if (virRandomBytes((unsigned char *) &val, sizeof(val)) < 0) {
/* You're already hosed, so this particular non-random value
* isn't any worse. */
return 0;
}
+ ret = virReadBufInt64LE(val);
+
if (nbits < 64)
ret &= (1ULL << nbits) - 1;
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Thursday, 2 August 2018 09:56:49 CEST Bjoern Walk wrote:
> 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>
> ---
> This goes on top of Michal's fix: https://www.redhat.com/archives/libvir-list/2018-August/msg00080.html
>
> src/util/virrandom.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virrandom.c b/src/util/virrandom.c
> index 7915f653..26ff68f5 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"
> @@ -61,13 +62,16 @@ VIR_LOG_INIT("util.random");
> uint64_t virRandomBits(int nbits)
> {
> uint64_t ret = 0;
> + uint8_t val[8];
>
> - if (virRandomBytes((unsigned char *) &ret, sizeof(ret)) < 0) {
> + if (virRandomBytes((unsigned char *) &val, sizeof(val)) < 0) {
> /* You're already hosed, so this particular non-random value
> * isn't any worse. */
> return 0;
> }
>
> + ret = virReadBufInt64LE(val);
> +
I do not think this patch is correct: we are dealing with random bytes,
so there is no "endianness" for them.
--
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-02, 10:02AM +0200]: > I do not think this patch is correct: we are dealing with random bytes, > so there is no "endianness" for them. Well, it's not incorrect either, isn't it? I agree that endianness doesn't matter for random data, but in the same time, it doesn't hurt to have the same random data generated on big- and little-endian machines. And it gives an easy and future-proof fix for the mocked tests. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Aug 02, 2018 at 10:17:32AM +0200, Bjoern Walk wrote: > Pino Toscano <ptoscano@redhat.com> [2018-08-02, 10:02AM +0200]: > > I do not think this patch is correct: we are dealing with random bytes, > > so there is no "endianness" for them. > > Well, it's not incorrect either, isn't it? I agree that endianness > doesn't matter for random data, but in the same time, it doesn't hurt to > have the same random data generated on big- and little-endian machines. > And it gives an easy and future-proof fix for the mocked tests. Lets just modijfy tests/virrandommock.c to add mocking of virRandomBits alongside virRandomBytes. 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
On Thursday, 2 August 2018 12:28:45 CEST Daniel P. Berrangé wrote: > On Thu, Aug 02, 2018 at 10:17:32AM +0200, Bjoern Walk wrote: > > Pino Toscano <ptoscano@redhat.com> [2018-08-02, 10:02AM +0200]: > > > I do not think this patch is correct: we are dealing with random bytes, > > > so there is no "endianness" for them. > > > > Well, it's not incorrect either, isn't it? I agree that endianness > > doesn't matter for random data, but in the same time, it doesn't hurt to > > have the same random data generated on big- and little-endian machines. Not sure I understand -- since it's random data, you cannot have it "the same", no matter which endianness the machine has. > > And it gives an easy and future-proof fix for the mocked tests. IMHO every mocked test has its own behaviour, and thus the mocking needs to reflect that. > Lets just modijfy tests/virrandommock.c to add mocking of virRandomBits > alongside virRandomBytes. I don't see how it will help, since all virRandomBits does is calling virRandomBytes. I still did not see any complaints about my patch to fix viriscsitest (since the problem is specific for it), what about ACKing it then? -- Pino Toscano-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Aug 02, 2018 at 01:12:02PM +0200, Pino Toscano wrote: > On Thursday, 2 August 2018 12:28:45 CEST Daniel P. Berrangé wrote: > > On Thu, Aug 02, 2018 at 10:17:32AM +0200, Bjoern Walk wrote: > > > Pino Toscano <ptoscano@redhat.com> [2018-08-02, 10:02AM +0200]: > > > > I do not think this patch is correct: we are dealing with random bytes, > > > > so there is no "endianness" for them. > > > > > > Well, it's not incorrect either, isn't it? I agree that endianness > > > doesn't matter for random data, but in the same time, it doesn't hurt to > > > have the same random data generated on big- and little-endian machines. > > Not sure I understand -- since it's random data, you cannot have it > "the same", no matter which endianness the machine has. > > > > And it gives an easy and future-proof fix for the mocked tests. > > IMHO every mocked test has its own behaviour, and thus the mocking > needs to reflect that. > > > Lets just modijfy tests/virrandommock.c to add mocking of virRandomBits > > alongside virRandomBytes. > > I don't see how it will help, since all virRandomBits does is calling > virRandomBytes. That's exactly what it would fix. A virRandomBits mock would simply return a fixed number and not call the mocked virRandomBytes at all thus avoiding the endianness problem > I still did not see any complaints about my patch to fix viriscsitest > (since the problem is specific for it), what about ACKing it then? The virrandommock.c was intended to provide "stable" random numbers across multiple tests. It didn't mock virRandomBits because it was mistakenly assumed that mocking virRandomBytes was enough. Changing virscsitest just fixes the one occurrance that happens to hit today - we want to fix the general problem as virrandommock.c intended. 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
© 2016 - 2024 Red Hat, Inc.