| Message ID | 1438849438-18552-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi, As "/dev/random" can be blocking perhaps it is possible/better to use the QEMU rng backend instead ? Laurent On 06/08/2015 10:23, Thomas Huth wrote: > The spapr firmware SLOF likely needs a way to get random numbers > soon. Instead of re-inventing the wheel there, we could simply > use the H_RANDOM hypercall to get the random numbers from QEMU. > For this the H_RANDOM hypercall needs to be implemented first, of > course. > So this patch introduces an OS-specifc helper function called > qemu_random() to get "good" random numbers (from /dev/random on > POSIX systems and via CryptGenRandom() on Windows), and then uses > this helper functions to provide random data to the guest via the > H_RANDOM hypercall. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > Note: Since I do not have access to the SPAPR spec, I figured out > the calling conventions of the hypercall by looking at the way > the Linux kernel is using this call to retriev random data. So > there might be some bugs or missing pieces here. > > Also note that the Windows version of qemu_random() is > compile-tested only (with a MinGW cross-compiler), since I do not > have access to a Windows system right now. > > hw/ppc/spapr_hcall.c | 12 ++++++++++++ > include/hw/ppc/spapr.h | 1 + > include/qemu/osdep.h | 11 +++++++++++ > util/oslib-posix.c | 16 ++++++++++++++++ > util/oslib-win32.c | 17 +++++++++++++++++ > 5 files changed, 57 insertions(+) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index fcfb50c..30f4359 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -931,6 +931,15 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > return H_SUCCESS; > } > > +static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr, > + target_ulong opcode, target_ulong *args) > +{ > + if (qemu_random(&args[0], sizeof(args[0]))) { > + return H_HARDWARE; > + } > + return H_SUCCESS; > +} > + > static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1]; > static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1]; > > @@ -1016,6 +1025,9 @@ static void hypercall_register_types(void) > > /* ibm,client-architecture-support support */ > spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support); > + > + /* misc */ > + spapr_register_hypercall(H_RANDOM, h_random); > } > > type_init(hypercall_register_types) > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 46e9704..232cda1 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -331,6 +331,7 @@ struct sPAPRMachineState { > #define H_SET_MPP 0x2D0 > #define H_GET_MPP 0x2D4 > #define H_XIRR_X 0x2FC > +#define H_RANDOM 0x300 > #define H_SET_MODE 0x31C > #define MAX_HCALL_OPCODE H_SET_MODE > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 3247364..1ef0f11 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -261,4 +261,15 @@ void os_mem_prealloc(int fd, char *area, size_t sz); > > int qemu_read_password(char *buf, int buf_size); > > +/** > + * qemu_random: > + * @buf: pointer to the buffer where the values should be stored > + * @size: amount of bytes that should be stored > + * > + * Retrieve random numbers, store them in @buf. > + * > + * Returns: 0 on success, error code otherwise > + */ > +int qemu_random(void *buf, int size); > + > #endif > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 3ae4987..43f5a51 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -482,3 +482,19 @@ int qemu_read_password(char *buf, int buf_size) > printf("\n"); > return ret; > } > + > +int qemu_random(void *buf, int size) > +{ > + FILE *fh; > + > + fh = fopen("/dev/random", "rb"); > + if (!fh) { > + return -errno; > + } > + if (fread(buf, 1, size, fh) != size) { > + return -EIO; > + } > + fclose(fh); > + > + return 0; > +} > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 730a670..63f0a94 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -494,3 +494,20 @@ int qemu_read_password(char *buf, int buf_size) > buf[i] = '\0'; > return 0; > } > + > +int qemu_random(void *buf, int size) > +{ > + HCRYPTPROV hprov = 0; > + int ret = 0; > + > + if (!CryptAcquireContext(&hprov, NULL, NULL, PROV_RSA_FULL, > + CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) { > + return -EINVAL; > + } > + if (!CryptGenRandom(hprov, size, buf)) { > + ret = -EIO; > + } > + CryptReleaseContext(hprov, 0); > + > + return ret; > +} >
On 06/08/15 13:26, Laurent Vivier wrote: > Hi, > > As "/dev/random" can be blocking perhaps it is possible/better to use > the QEMU rng backend instead ? Actually, I tried to use the rng backends first ... but they turned out to be horrible for being used in a synchronous call like I need here: You've got to install a callback function which is called at a "random" point in time when some data becomes available - and it is even called with less bytes than you requested! (i.e. I requested 8 bytes, but the callback got only called with 6 bytes). So to use it, I'd have to introduce a ring buffer or something similar to query enough random data in advance - and when it runs empty, the H_RANDOM hypercall would need to block again anyway. So instead of introducing a hard-to-understand-and-maintain "monster wrapper" around the rng backend functions, I think the short and easy understandable qemu_random() implementation in this patch here is the better choice. And looking at https://patchwork.ozlabs.org/patch/497062/ I think it also should not hurt too much that qemu_random() might be slow here since the real hardware number generator is apparently also not very fast. Thomas > On 06/08/2015 10:23, Thomas Huth wrote: >> The spapr firmware SLOF likely needs a way to get random numbers >> soon. Instead of re-inventing the wheel there, we could simply >> use the H_RANDOM hypercall to get the random numbers from QEMU. >> For this the H_RANDOM hypercall needs to be implemented first, of >> course. >> So this patch introduces an OS-specifc helper function called >> qemu_random() to get "good" random numbers (from /dev/random on >> POSIX systems and via CryptGenRandom() on Windows), and then uses >> this helper functions to provide random data to the guest via the >> H_RANDOM hypercall. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> Note: Since I do not have access to the SPAPR spec, I figured out >> the calling conventions of the hypercall by looking at the way >> the Linux kernel is using this call to retriev random data. So >> there might be some bugs or missing pieces here. >> >> Also note that the Windows version of qemu_random() is >> compile-tested only (with a MinGW cross-compiler), since I do not >> have access to a Windows system right now. >> >> hw/ppc/spapr_hcall.c | 12 ++++++++++++ >> include/hw/ppc/spapr.h | 1 + >> include/qemu/osdep.h | 11 +++++++++++ >> util/oslib-posix.c | 16 ++++++++++++++++ >> util/oslib-win32.c | 17 +++++++++++++++++ >> 5 files changed, 57 insertions(+)
On 06/08/2015 14:05, Thomas Huth wrote: > On 06/08/15 13:26, Laurent Vivier wrote: >> Hi, >> >> As "/dev/random" can be blocking perhaps it is possible/better to use >> the QEMU rng backend instead ? > > Actually, I tried to use the rng backends first ... but they turned out > to be horrible for being used in a synchronous call like I need here: > You've got to install a callback function which is called at a "random" > point in time when some data becomes available - and it is even called > with less bytes than you requested! (i.e. I requested 8 bytes, but the > callback got only called with 6 bytes). So to use it, I'd have to > introduce a ring buffer or something similar to query enough random data > in advance - and when it runs empty, the H_RANDOM hypercall would need > to block again anyway. > > So instead of introducing a hard-to-understand-and-maintain "monster > wrapper" around the rng backend functions, I think the short and easy > understandable qemu_random() implementation in this patch here is the > better choice. > > And looking at https://patchwork.ozlabs.org/patch/497062/ I think it > also should not hurt too much that qemu_random() might be slow here > since the real hardware number generator is apparently also not very fast. It seems a good choice... Why do you use buffered file descriptor, fopen/fread/fclose instead of open/read/close ? Laurent
On 06/08/15 14:35, Laurent Vivier wrote: > > > On 06/08/2015 14:05, Thomas Huth wrote: >> On 06/08/15 13:26, Laurent Vivier wrote: >>> Hi, >>> >>> As "/dev/random" can be blocking perhaps it is possible/better to use >>> the QEMU rng backend instead ? >> >> Actually, I tried to use the rng backends first ... but they turned out >> to be horrible for being used in a synchronous call like I need here: >> You've got to install a callback function which is called at a "random" >> point in time when some data becomes available - and it is even called >> with less bytes than you requested! (i.e. I requested 8 bytes, but the >> callback got only called with 6 bytes). So to use it, I'd have to >> introduce a ring buffer or something similar to query enough random data >> in advance - and when it runs empty, the H_RANDOM hypercall would need >> to block again anyway. >> >> So instead of introducing a hard-to-understand-and-maintain "monster >> wrapper" around the rng backend functions, I think the short and easy >> understandable qemu_random() implementation in this patch here is the >> better choice. >> >> And looking at https://patchwork.ozlabs.org/patch/497062/ I think it >> also should not hurt too much that qemu_random() might be slow here >> since the real hardware number generator is apparently also not very fast. > > It seems a good choice... > > Why do you use buffered file descriptor, fopen/fread/fclose instead of > open/read/close ? You mean to avoid that fread gets some additional bytes in advance? ... that might be a good idea, thanks, I'll change my patch accordingly. Thomas
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index fcfb50c..30f4359 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -931,6 +931,15 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, return H_SUCCESS; } +static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr, + target_ulong opcode, target_ulong *args) +{ + if (qemu_random(&args[0], sizeof(args[0]))) { + return H_HARDWARE; + } + return H_SUCCESS; +} + static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1]; static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1]; @@ -1016,6 +1025,9 @@ static void hypercall_register_types(void) /* ibm,client-architecture-support support */ spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support); + + /* misc */ + spapr_register_hypercall(H_RANDOM, h_random); } type_init(hypercall_register_types) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 46e9704..232cda1 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -331,6 +331,7 @@ struct sPAPRMachineState { #define H_SET_MPP 0x2D0 #define H_GET_MPP 0x2D4 #define H_XIRR_X 0x2FC +#define H_RANDOM 0x300 #define H_SET_MODE 0x31C #define MAX_HCALL_OPCODE H_SET_MODE diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 3247364..1ef0f11 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -261,4 +261,15 @@ void os_mem_prealloc(int fd, char *area, size_t sz); int qemu_read_password(char *buf, int buf_size); +/** + * qemu_random: + * @buf: pointer to the buffer where the values should be stored + * @size: amount of bytes that should be stored + * + * Retrieve random numbers, store them in @buf. + * + * Returns: 0 on success, error code otherwise + */ +int qemu_random(void *buf, int size); + #endif diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 3ae4987..43f5a51 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -482,3 +482,19 @@ int qemu_read_password(char *buf, int buf_size) printf("\n"); return ret; } + +int qemu_random(void *buf, int size) +{ + FILE *fh; + + fh = fopen("/dev/random", "rb"); + if (!fh) { + return -errno; + } + if (fread(buf, 1, size, fh) != size) { + return -EIO; + } + fclose(fh); + + return 0; +} diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 730a670..63f0a94 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -494,3 +494,20 @@ int qemu_read_password(char *buf, int buf_size) buf[i] = '\0'; return 0; } + +int qemu_random(void *buf, int size) +{ + HCRYPTPROV hprov = 0; + int ret = 0; + + if (!CryptAcquireContext(&hprov, NULL, NULL, PROV_RSA_FULL, + CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) { + return -EINVAL; + } + if (!CryptGenRandom(hprov, size, buf)) { + ret = -EIO; + } + CryptReleaseContext(hprov, 0); + + return ret; +}
The spapr firmware SLOF likely needs a way to get random numbers soon. Instead of re-inventing the wheel there, we could simply use the H_RANDOM hypercall to get the random numbers from QEMU. For this the H_RANDOM hypercall needs to be implemented first, of course. So this patch introduces an OS-specifc helper function called qemu_random() to get "good" random numbers (from /dev/random on POSIX systems and via CryptGenRandom() on Windows), and then uses this helper functions to provide random data to the guest via the H_RANDOM hypercall. Signed-off-by: Thomas Huth <thuth@redhat.com> --- Note: Since I do not have access to the SPAPR spec, I figured out the calling conventions of the hypercall by looking at the way the Linux kernel is using this call to retriev random data. So there might be some bugs or missing pieces here. Also note that the Windows version of qemu_random() is compile-tested only (with a MinGW cross-compiler), since I do not have access to a Windows system right now. hw/ppc/spapr_hcall.c | 12 ++++++++++++ include/hw/ppc/spapr.h | 1 + include/qemu/osdep.h | 11 +++++++++++ util/oslib-posix.c | 16 ++++++++++++++++ util/oslib-win32.c | 17 +++++++++++++++++ 5 files changed, 57 insertions(+)