On Fri, Apr 29, 2022 at 6:04 PM Denys Vlasenko <vda.li...@googlemail.com> wrote:
> On Wed, Apr 27, 2022 at 6:55 PM Jason A. Donenfeld <ja...@zx2c4.com> wrote:
> > On Wed, Apr 27, 2022 at 06:15:50PM +0200, Denys Vlasenko wrote:
> > > if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) {
> > > bb_perror_msg("can't%s seed", " remove");
> > > return -1;
> > > }
> > >
> > > Why can't the entire above if() be replaced with xunlink(filename) ?
> >
> > It cannot be replaced with that. The fsync() must happen before other
> > operations move forward,
>
> Why? It should be explained in a comment, and explained well enough
> so that future developers do not repeatedly try to remove it
> because "I don't see why it's necessary".
>
> > and exiting the program isn't necessarily the
> > correct course of action.
>
> Let's see.
> If we opened the file, read it, and then failed to unlink it,
> it means that filesystem is read-only, there is a permission problem
> (e.g. SELinux), or there is an I/O error.
> All of these cases mean that the system is not in expected state,
> and further efforts to "continue as normal" (e.g. try to rewrite
> /var/lib/seedrng/seed.credit) do not seem likely to be productive to me.
> I think it's better to just complain and exit.
Even partial removal of these complicated error paths
cuts down the size by ~10%
function old new delta
.rodata 104946 104938 -8
seedrng_main 1225 1077 -148
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-156) Total: -156 bytes
diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c
index c42274759..82c69b72b 100644
--- a/util-linux/seedrng.c
+++ b/util-linux/seedrng.c
@@ -100,63 +100,43 @@ static int read_new_seed(uint8_t *seed, size_t
len, bool *is_creditable)
return -1;
}
-static int seed_rng(uint8_t *seed, size_t len, bool credit)
+static void seed_rng(uint8_t *seed, size_t len, bool credit)
{
struct {
int entropy_count;
int buf_size;
uint8_t buffer[MAX_SEED_LEN];
} req;
- int random_fd, ret;
-
- if (len > sizeof(req.buffer)) {
- errno = EFBIG;
- return -1;
- }
+ int random_fd;
req.entropy_count = credit ? len * 8 : 0;
req.buf_size = len;
memcpy(req.buffer, seed, len);
- random_fd = open("/dev/urandom", O_RDONLY);
- if (random_fd < 0)
- return -1;
- ret = ioctl(random_fd, RNDADDENTROPY, &req);
- if (ret)
- ret = -errno ? -errno : -EIO;
+ random_fd = xopen("/dev/urandom", O_RDONLY);
+ xioctl(random_fd, RNDADDENTROPY, &req);
if (ENABLE_FEATURE_CLEAN_UP)
close(random_fd);
- errno = -ret;
- return ret ? -1 : 0;
}
-static int seed_from_file_if_exists(const char *filename, int dfd,
bool credit, sha256_ctx_t *hash)
+static void seed_from_file_if_exists(const char *filename, bool
credit, sha256_ctx_t *hash)
{
uint8_t seed[MAX_SEED_LEN];
ssize_t seed_len;
seed_len = open_read_close(filename, seed, sizeof(seed));
if (seed_len < 0) {
- if (errno == ENOENT)
- return 0;
- bb_perror_msg("can't%s seed", " read");
- return -1;
+ if (errno != ENOENT)
+ bb_perror_msg_and_die("can't%s seed", " read");
+ return;
}
- if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) {
- bb_perror_msg("can't%s seed", " remove");
- return -1;
- } else if (!seed_len)
- return 0;
-
- sha256_hash(hash, &seed_len, sizeof(seed_len));
- sha256_hash(hash, seed, seed_len);
-
- printf("Seeding %u bits %s crediting\n", (unsigned)seed_len * 8,
credit ? "and" : "without");
- if (seed_rng(seed, seed_len, credit) < 0) {
- bb_perror_msg("can't%s seed", "");
- return -1;
+ xunlink(filename);
+ if (seed_len != 0) {
+ sha256_hash(hash, &seed_len, sizeof(seed_len));
+ sha256_hash(hash, seed, seed_len);
+ printf("Seeding %u bits %s crediting\n", (unsigned)seed_len *
8, credit ? "and" : "without");
+ seed_rng(seed, seed_len, credit);
}
- return 0;
}
int seedrng_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE;
@@ -202,11 +182,9 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[])
sha256_hash(&hash, ×tamp, sizeof(timestamp));
for (int i = 1; i < 3; ++i) {
- if (seed_from_file_if_exists(i == 1 ?
NON_CREDITABLE_SEED_NAME : CREDITABLE_SEED_NAME,
- dfd,
+ seed_from_file_if_exists(i == 1 ? NON_CREDITABLE_SEED_NAME :
CREDITABLE_SEED_NAME,
i == 1 ? false : !skip_credit,
- &hash) < 0)
- program_ret |= 1 << i;
+ &hash);
}
new_seed_len = determine_optimal_seed_len();
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
