diff options
| author | Leah Rowe <leah@libreboot.org> | 2026-03-14 22:11:32 +0000 |
|---|---|---|
| committer | Leah Rowe <leah@libreboot.org> | 2026-03-14 22:15:33 +0000 |
| commit | f229c722a93c05abcc4cd544681d8d0cd1cfe204 (patch) | |
| tree | 87aebbd967ff280b499e5ae009a35362bfa9d390 | |
| parent | d66ba5f7817a3e07385e470cd11ba4eb090f3061 (diff) | |
util/nvmutil: partially mitigate fd offset race
our fallback pwrite/pread behaviour still does not
properly replicate the safety of real pwrite/pread
i intend to put this i/o code into a library for use
in other programs; nvmutil is single-threaded so
this change is largely redundant (but can't hurt)
Signed-off-by: Leah Rowe <leah@libreboot.org>
| -rw-r--r-- | util/nvmutil/nvmutil.c | 54 |
1 files changed, 42 insertions, 12 deletions
diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c index 7ece1764..0f5b7592 100644 --- a/util/nvmutil/nvmutil.c +++ b/util/nvmutil/nvmutil.c @@ -1951,6 +1951,7 @@ prw(int fd, void *mem, size_t nrw, int saved_errno; int positional_rw; struct stat st; + off_t verified; if (mem == NULL) goto err_prw; @@ -2014,22 +2015,51 @@ real_pread_pwrite: goto real_pread_pwrite; #else if ((off_orig = lseek_loop(fd, (off_t)0, SEEK_CUR, - loop_eagain, loop_eintr)) == (off_t)-1) + loop_eagain, loop_eintr)) == (off_t)-1) { r = -1; - else if (lseek_loop(fd, off, SEEK_SET, - loop_eagain, loop_eintr) == (off_t)-1) + } else if (lseek_loop(fd, off, SEEK_SET, + loop_eagain, loop_eintr) == (off_t)-1) { r = -1; + } else { + verified = lseek_loop(fd, (off_t)0, SEEK_CUR, + loop_eagain, loop_eintr); - do { - if (rw_type == IO_PREAD) - r = read(fd, mem, nrw); - else if (rw_type == IO_PWRITE) - r = write(fd, mem, nrw); + /* + * Partial thread-safety: detect + * if the offset changed to what + * we previously got. If it did, + * then another thread may have + * changed it. + * + * This is no substitute for real + * pread/pwrite, which would be + * fully atomic at kernel-level + * and do not use file offsets. + * + * TODO: Add a toggle to make it + * recover instead, reset + * to known offset, and + * carry on operations. + * + * Failure is the better option + * here, since recovery would + * mask hidden bugs in code. + */ + if (off != verified) + goto err_prw; - r = rw_over_nrw(r, nrw); - } while (r == -1 && - (errno == try_err(loop_eintr, EINTR) - || errno == try_err(loop_eagain, EAGAIN))); + do { + if (rw_type == IO_PREAD) + r = read(fd, mem, nrw); + else if (rw_type == IO_PWRITE) + r = write(fd, mem, nrw); + + r = rw_over_nrw(r, nrw); + + } while (r == -1 && + (errno == try_err(loop_eintr, EINTR) + || errno == try_err(loop_eagain, EAGAIN))); + } saved_errno = errno; off_last = lseek_loop(fd, off_orig, SEEK_SET, |
