Re: [libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
On 08/01/2018 07:16 AM, Daniel P. Berrangé wrote: On Wed, Aug 01, 2018 at 01:44:32PM +0200, Michal Privoznik wrote: The function is supposed to return up to 64bit long integer. In order to do that it calls virRandomBytes() to fill the integer with random bytes and then masks out everything but requested bits. However, when doing that it shifts 1U and not 1ULL. So effectively, requesting 32 random bis or more always return 0 which is not random enough. Signed-off-by: Michal Privoznik --- src/util/virrandom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 01cc82a052..3c011a8615 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) return 0; } -ret &= (1U << nbits) - 1; +ret &= (1ULL << nbits) - 1; 1ULL << 64 is undefined in C. We need to write this as: if (nbits < 64) ret &= (1ULL << nbits) - 1; -- 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
Re: [libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
On 08/01/2018 04:50 PM, Eric Blake wrote: > On 08/01/2018 07:16 AM, Daniel P. Berrangé wrote: >> On Wed, Aug 01, 2018 at 01:44:32PM +0200, Michal Privoznik wrote: >>> The function is supposed to return up to 64bit long integer. In >>> order to do that it calls virRandomBytes() to fill the integer >>> with random bytes and then masks out everything but requested >>> bits. However, when doing that it shifts 1U and not 1ULL. So >>> effectively, requesting 32 random bis or more always return 0 >>> which is not random enough. >>> >>> Signed-off-by: Michal Privoznik >>> --- >>> src/util/virrandom.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/util/virrandom.c b/src/util/virrandom.c >>> index 01cc82a052..3c011a8615 100644 >>> --- a/src/util/virrandom.c >>> +++ b/src/util/virrandom.c >>> @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) >>> return 0; >>> } >>> - ret &= (1U << nbits) - 1; >>> + ret &= (1ULL << nbits) - 1; > > 1ULL << 64 is undefined in C. We need to write this as: > > if (nbits < 64) > ret &= (1ULL << nbits) - 1; > > Oops. okay. I'll post a patch for that since this is pushed already. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
On Wed, Aug 01, 2018 at 01:44:32PM +0200, Michal Privoznik wrote: > The function is supposed to return up to 64bit long integer. In > order to do that it calls virRandomBytes() to fill the integer > with random bytes and then masks out everything but requested > bits. However, when doing that it shifts 1U and not 1ULL. So > effectively, requesting 32 random bis or more always return 0 > which is not random enough. > > Signed-off-by: Michal Privoznik > --- > src/util/virrandom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/util/virrandom.c b/src/util/virrandom.c > index 01cc82a052..3c011a8615 100644 > --- a/src/util/virrandom.c > +++ b/src/util/virrandom.c > @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) > return 0; > } > > -ret &= (1U << nbits) - 1; > +ret &= (1ULL << nbits) - 1; > return ret; > } Reviewed-by: Daniel P. Berrangé 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
[libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
The function is supposed to return up to 64bit long integer. In order to do that it calls virRandomBytes() to fill the integer with random bytes and then masks out everything but requested bits. However, when doing that it shifts 1U and not 1ULL. So effectively, requesting 32 random bis or more always return 0 which is not random enough. Signed-off-by: Michal Privoznik --- src/util/virrandom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 01cc82a052..3c011a8615 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) return 0; } -ret &= (1U << nbits) - 1; +ret &= (1ULL << nbits) - 1; return ret; } -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
