Hello Jason, Am Fri, Apr 29, 2022 at 07:04:47PM +0200 schrieb Jason A. Donenfeld: > Rather than having getrandom() be called in a loop that handles EINTR -- > which would require more code bloat -- we just limit the maximum seed > size to 256 bytes, which the kernel guarantees won't be interrupted. > Additionally document the flock() and fsync() usage so that somebody > doesn't remove it. Apparently busybox developers like to remove things > they don't understand with no regards to security implications, so Denys > suggested I leave some comments here. > > Cc: Denys Vlasenko <vda.li...@googlemail.com> > Cc: Bernhard Reutner-Fischer <rep.dot....@gmail.com> > Signed-off-by: Jason A. Donenfeld <ja...@zx2c4.com> > --- > util-linux/seedrng.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c > index c42274759..5ee6197e3 100644 > --- a/util-linux/seedrng.c > +++ b/util-linux/seedrng.c > @@ -56,7 +56,7 @@ > > enum { > MIN_SEED_LEN = SHA256_OUTSIZE, > - MAX_SEED_LEN = 512 > + MAX_SEED_LEN = 256 /* Maximum size of getrandom() call without EINTR. */ > }; > > static size_t determine_optimal_seed_len(void) > @@ -142,6 +142,8 @@ static int seed_from_file_if_exists(const char *filename, > int dfd, bool credit, > bb_perror_msg("can't%s seed", " read"); > return -1; > } > + /* The fsync() here is necessary for safety here, so that power being > pulled > + * at the wrong moment doesn't result in the seed being used twice by > accident. */
nit: "here" is used twice in the same sentence. Greets Alex > if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) { > bb_perror_msg("can't%s seed", " remove"); > return -1; > @@ -190,6 +192,8 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) > if (mkdir(seed_dir, 0700) < 0 && errno != EEXIST) > bb_perror_msg_and_die("can't %s seed directory", "create"); > dfd = open(seed_dir, O_DIRECTORY | O_RDONLY); > + /* The flock() here is absolutely necessary, as the consistency of this > + * program breaks down with concurrent uses. */ > if (dfd < 0 || flock(dfd, LOCK_EX) < 0) > bb_perror_msg_and_die("can't %s seed directory", "lock"); > xfchdir(dfd); > @@ -222,6 +226,8 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) > > printf("Saving %u bits of %screditable seed for next boot\n", > (unsigned)new_seed_len * 8, new_seed_creditable ? "" : "non-"); > fd = open(NON_CREDITABLE_SEED_NAME, O_WRONLY | O_CREAT | O_TRUNC, 0400); > + /* The fsync() here is necessary to ensure the data is written to disk > before > + * we attempt to make it creditable. */ > if (fd < 0 || full_write(fd, new_seed, new_seed_len) != > (ssize_t)new_seed_len || fsync(fd) < 0) { > bb_perror_msg("can't%s seed", " write"); > return program_ret | (1 << 4); > -- > 2.35.1 > > _______________________________________________ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox