| Message ID | 20190315032629.21234-13-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add qemu_getrandom and ARMv8.5-RNG etc | expand |
Hi Richard, On 3/15/19 4:26 AM, Richard Henderson wrote: > Cc: Laurent Vivier <laurent@vivier.eu> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/main.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index cf7095bdaf..8478306eef 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -38,6 +38,7 @@ > #include "trace/control.h" > #include "target_elf.h" > #include "cpu_loop-common.h" > +#include "crypto/init.h" > > char *exec_path; > > @@ -686,8 +687,18 @@ int main(int argc, char **argv, char **envp) > if (seed_optarg == NULL) { > seed_optarg = getenv("QEMU_RAND_SEED"); > } > - if (seed_optarg != NULL) { > - qemu_guest_random_seed_main(seed_optarg, &error_fatal); > + { Since 7be41675f7c we use gnu99 C, so this extra block indentation can be removed. > + Error *err = NULL; > + if (seed_optarg != NULL) { > + qemu_guest_random_seed_main(seed_optarg, &err); > + } else { > + /* ??? Assumes qcrypto is only used by qemu_guest_getrandom. */ > + qcrypto_init(&err); > + } > + if (err) { > + error_reportf_err(err, "cannot initialize crypto: "); > + exit(1); > + } > } > > target_environ = envlist_to_environ(envlist, NULL); >
On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote: >> - if (seed_optarg != NULL) { >> - qemu_guest_random_seed_main(seed_optarg, &error_fatal); >> + { > Since 7be41675f7c we use gnu99 C, so this extra block indentation can be > removed. > >> + Error *err = NULL; >> + if (seed_optarg != NULL) { >> + qemu_guest_random_seed_main(seed_optarg, &err); >> + } else { >> + /* ??? Assumes qcrypto is only used by qemu_guest_getrandom. */ >> + qcrypto_init(&err); >> + } >> + if (err) { >> + error_reportf_err(err, "cannot initialize crypto: "); >> + exit(1); >> + } >> } I could, but it also limits the scope, which is of more importance to variables who have their address taken. It means that their storage could (in theory) be shared with objects not overlapping in scope. r~
On 4/13/19 8:44 AM, Richard Henderson wrote: > On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote: >>> - if (seed_optarg != NULL) { >>> - qemu_guest_random_seed_main(seed_optarg, &error_fatal); >>> + { >> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be >> removed. >> >>> + Error *err = NULL; >>> + if (seed_optarg != NULL) { >>> + qemu_guest_random_seed_main(seed_optarg, &err); >>> + } else { >>> + /* ??? Assumes qcrypto is only used by qemu_guest_getrandom. */ >>> + qcrypto_init(&err); >>> + } >>> + if (err) { >>> + error_reportf_err(err, "cannot initialize crypto: "); >>> + exit(1); >>> + } >>> } > > I could, but it also limits the scope, which is of more importance to variables > who have their address taken. It means that their storage could (in theory) be > shared with objects not overlapping in scope. Fine then. I think your '???' comment is appropriate but I'd rather let Daniel opinate. Except that comment, for the rest: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Mon, Apr 15, 2019 at 12:23:18PM +0200, Philippe Mathieu-Daudé wrote: > On 4/13/19 8:44 AM, Richard Henderson wrote: > > On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote: > >>> - if (seed_optarg != NULL) { > >>> - qemu_guest_random_seed_main(seed_optarg, &error_fatal); > >>> + { > >> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be > >> removed. > >> > >>> + Error *err = NULL; > >>> + if (seed_optarg != NULL) { > >>> + qemu_guest_random_seed_main(seed_optarg, &err); > >>> + } else { > >>> + /* ??? Assumes qcrypto is only used by qemu_guest_getrandom. */ > >>> + qcrypto_init(&err); > >>> + } > >>> + if (err) { > >>> + error_reportf_err(err, "cannot initialize crypto: "); > >>> + exit(1); > >>> + } > >>> } > > > > I could, but it also limits the scope, which is of more importance to variables > > who have their address taken. It means that their storage could (in theory) be > > shared with objects not overlapping in scope. > > Fine then. > > I think your '???' comment is appropriate but I'd rather let Daniel > opinate. Except that comment, for the rest: In linux-user context, afaik, the random APIs are the only stuff that will be used, none of the hash or cipher stuff is needed. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@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 :|
diff --git a/linux-user/main.c b/linux-user/main.c index cf7095bdaf..8478306eef 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -38,6 +38,7 @@ #include "trace/control.h" #include "target_elf.h" #include "cpu_loop-common.h" +#include "crypto/init.h" char *exec_path; @@ -686,8 +687,18 @@ int main(int argc, char **argv, char **envp) if (seed_optarg == NULL) { seed_optarg = getenv("QEMU_RAND_SEED"); } - if (seed_optarg != NULL) { - qemu_guest_random_seed_main(seed_optarg, &error_fatal); + { + Error *err = NULL; + if (seed_optarg != NULL) { + qemu_guest_random_seed_main(seed_optarg, &err); + } else { + /* ??? Assumes qcrypto is only used by qemu_guest_getrandom. */ + qcrypto_init(&err); + } + if (err) { + error_reportf_err(err, "cannot initialize crypto: "); + exit(1); + } } target_environ = envlist_to_environ(envlist, NULL);
Cc: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/main.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) -- 2.17.2