From 898f916778ef727c747ed26c49de4e8ee2bd78b8 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 7 Jun 2024 19:00:35 -0500 Subject: [PATCH] Fixed pl hole in perturb logic Turns out there's very _very_ small powerloss hole in our current perturb logic. We rely on tag valid bits to validate perturb bits, but these intentionally don't end up in the commit checksum. This means there will always be a powerloss hole when we write the last valid bit. If we lose power after writing that bit, suddenly the remaining commit and any following commits may appear as valid. Now, this is really unlikely considering we need to lose power exactly when we write the cksum tag's valid bit, and our nonce helps protect against this. But a hole is a hole. The solution here is to include the _current_ perturb bit (q) in the commit's cksum tag, alongside the _next_ perturb bit (p). This will be included in the commit's checksum, but _not_ in the canonical checksum, allowing the commit's checksum validate the current perturb state without ruining our erased-state agnostic checksums: .---+---+---+---. . . .---+---+---+---. \ \ \ \ |v| tag | |v| tag | | | | | +---+---+---+---+ +---+---+---+---+ | | | | | commit | | commit | | | | | | | | | +-. | | | +---+---+---+---+ +---+---+---+---+ / | | | | |v|qp-------------. |v|qp| tag | | . . . +---+---+---+---+ | +---+---+---+---+ | . . . | cksum | | | cksum | | . . . +---+---+---+---+ | +---+---+---+---+ | . . . | padding | | | padding | | . . . | | | | | | . . . +---+---+---+---+ | . +---+---+---+---+ | | | | | erased | +-> |v------------------' | | | | | | +---+---+---+---+ | | | . . | | commit | +-. | +- rbyd . . | |.----------------. | | | | cksum | +| -+---+---+---+ | / | +-. / +-> |v|qp| tag | '-----' | | | +- ^ ---+---+---+ / | '------' cksum ----------------' +---+---+---+---+ | padding | | | +---+---+---+---+ | erased | | | . . . . (Ok maybe this diagram needs work...) This adds another thing that needs to be checked during rbyd fetch, and note, we _do_ need to explicitly check this, but it solves the problem. If power is loss after v, q would be invalid, and if power is lost after q, our cksum would be invalid. Note this would have also been an issue for the previous cksum + parity perturb scheme. Code changes: code stack before: 33570 2592 after: 33598 (+0.1%) 2592 (+0.0%) --- lfs.c | 20 ++++++++++++++++---- scripts/dbgbmap.py | 6 +++++- scripts/dbgbtree.py | 14 +++++++++----- scripts/dbglfs.py | 14 +++++++++----- scripts/dbgmtree.py | 14 +++++++++----- scripts/dbgrbyd.py | 18 +++++++++++++----- scripts/dbgtag.py | 10 +++++----- 7 files changed, 66 insertions(+), 30 deletions(-) diff --git a/lfs.c b/lfs.c index b9e3e847..893d9ab4 100644 --- a/lfs.c +++ b/lfs.c @@ -830,8 +830,8 @@ enum lfsr_tag { // checksum tags LFSR_TAG_CKSUM = 0x3000, - LFSR_TAG_Q = 0x0000, LFSR_TAG_P = 0x0001, + LFSR_TAG_Q = 0x0002, LFSR_TAG_NOISE = 0x3100, LFSR_TAG_ECKSUM = 0x3200, @@ -902,10 +902,14 @@ static inline bool lfsr_tag_istrunk(lfsr_tag_t tag) { return lfsr_tag_mode(tag) != LFSR_TAG_CKSUM; } -static inline bool lfsr_tag_perturb(lfsr_tag_t tag) { +static inline bool lfsr_tag_p(lfsr_tag_t tag) { return tag & LFSR_TAG_P; } +static inline bool lfsr_tag_q(lfsr_tag_t tag) { + return tag & LFSR_TAG_Q; +} + static inline bool lfsr_tag_isinternal(lfsr_tag_t tag) { return tag & LFSR_TAG_INTERNAL; } @@ -2213,6 +2217,12 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd, // is an end-of-commit cksum } else { + // check perturb bit + if (lfsr_rbyd_perturb(rbyd) != lfsr_tag_q(tag)) { + // uh oh, perturb bits don't match + break; + } + // check cksum uint32_t cksum__ = 0; err = lfsr_bd_read(lfs, block, off_, -1, @@ -2232,7 +2242,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd, // save what we've found so far rbyd->eoff - = ((lfs_size_t)lfsr_tag_perturb(tag) + = ((lfs_size_t)lfsr_tag_p(tag) << (8*sizeof(lfs_size_t)-1)) | (off_ + size); rbyd->cksum = cksum; @@ -3423,8 +3433,10 @@ static int lfsr_rbyd_appendcksum(lfs_t *lfs, lfsr_rbyd_t *rbyd) { // set the valid bit to the cksum parity | ((uint8_t)lfs_parity(rbyd->cksum) << 7); cksum_buf[1] = (uint8_t)(LFSR_TAG_CKSUM >> 0) + // include the current perturb bit + | ((uint8_t)lfsr_rbyd_perturb(rbyd) << 1) // set the perturb bit so next commit is invalid - | perturb; + | ((uint8_t)perturb << 0); cksum_buf[2] = 0; lfs_size_t padding = off_ - (lfsr_rbyd_eoff(rbyd) + 2+1+4); diff --git a/scripts/dbgbmap.py b/scripts/dbgbmap.py index 2a2b5423..d29bc640 100755 --- a/scripts/dbgbmap.py +++ b/scripts/dbgbmap.py @@ -46,8 +46,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -683,6 +683,10 @@ class Rbyd: cksum__ = crc32c(data[j_:j_+size], cksum__) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + break + # check cksum cksum___ = fromle32(data[j_:j_+4]) if cksum__ != cksum___: break diff --git a/scripts/dbgbtree.py b/scripts/dbgbtree.py index 33501353..d3ec5dd9 100755 --- a/scripts/dbgbtree.py +++ b/scripts/dbgbtree.py @@ -44,8 +44,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -239,10 +239,10 @@ def tagrepr(tag, w=None, size=None, off=None): else ' -%d' % size if size else '') elif (tag & 0x7f00) == TAG_CKSUM: - return 'cksum%s%s%s' % ( - 'p' if tag & 0xff == TAG_P - else 'q' if tag & 0xff == TAG_Q - else ' 0x%02x' % (tag & 0xff), + return 'cksum%s%s%s%s%s' % ( + 'q' if not tag & 0xfc and tag & TAG_Q else '', + 'p' if not tag & 0xfc and tag & TAG_P else '', + ' 0x%02x' % (tag & 0xff) if tag & 0xfc else '', ' w%d' % w if w else '', ' %s' % size if size is not None else '') elif (tag & 0x7f00) == TAG_NOISE: @@ -356,6 +356,10 @@ class Rbyd: cksum__ = crc32c(data[j_:j_+size], cksum__) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + break + # check cksum cksum___ = fromle32(data[j_:j_+4]) if cksum__ != cksum___: break diff --git a/scripts/dbglfs.py b/scripts/dbglfs.py index 09b2bee9..f3274750 100755 --- a/scripts/dbglfs.py +++ b/scripts/dbglfs.py @@ -45,8 +45,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -270,10 +270,10 @@ def tagrepr(tag, w=None, size=None, off=None): else ' -%d' % size if size else '') elif (tag & 0x7f00) == TAG_CKSUM: - return 'cksum%s%s%s' % ( - 'p' if tag & 0xff == TAG_P - else 'q' if tag & 0xff == TAG_Q - else ' 0x%02x' % (tag & 0xff), + return 'cksum%s%s%s%s%s' % ( + 'q' if not tag & 0xfc and tag & TAG_Q else '', + 'p' if not tag & 0xfc and tag & TAG_P else '', + ' 0x%02x' % (tag & 0xff) if tag & 0xfc else '', ' w%d' % w if w else '', ' %s' % size if size is not None else '') elif (tag & 0x7f00) == TAG_NOISE: @@ -387,6 +387,10 @@ class Rbyd: cksum__ = crc32c(data[j_:j_+size], cksum__) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + break + # check cksum cksum___ = fromle32(data[j_:j_+4]) if cksum__ != cksum___: break diff --git a/scripts/dbgmtree.py b/scripts/dbgmtree.py index 2c69aa1c..2b2dc197 100755 --- a/scripts/dbgmtree.py +++ b/scripts/dbgmtree.py @@ -44,8 +44,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -254,10 +254,10 @@ def tagrepr(tag, w=None, size=None, off=None): else ' -%d' % size if size else '') elif (tag & 0x7f00) == TAG_CKSUM: - return 'cksum%s%s%s' % ( - 'p' if tag & 0xff == TAG_P - else 'q' if tag & 0xff == TAG_Q - else ' 0x%02x' % (tag & 0xff), + return 'cksum%s%s%s%s%s' % ( + 'q' if not tag & 0xfc and tag & TAG_Q else '', + 'p' if not tag & 0xfc and tag & TAG_P else '', + ' 0x%02x' % (tag & 0xff) if tag & 0xfc else '', ' w%d' % w if w else '', ' %s' % size if size is not None else '') elif (tag & 0x7f00) == TAG_NOISE: @@ -371,6 +371,10 @@ class Rbyd: cksum__ = crc32c(data[j_:j_+size], cksum__) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + break + # check cksum cksum___ = fromle32(data[j_:j_+4]) if cksum__ != cksum___: break diff --git a/scripts/dbgrbyd.py b/scripts/dbgrbyd.py index 580a3448..88d535b7 100755 --- a/scripts/dbgrbyd.py +++ b/scripts/dbgrbyd.py @@ -53,8 +53,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -241,10 +241,10 @@ def tagrepr(tag, w=None, size=None, off=None): else ' -%d' % size if size else '') elif (tag & 0x7f00) == TAG_CKSUM: - return 'cksum%s%s%s' % ( - 'p' if tag & 0xff == TAG_P - else 'q' if tag & 0xff == TAG_Q - else ' 0x%02x' % (tag & 0xff), + return 'cksum%s%s%s%s%s' % ( + 'q' if not tag & 0xfc and tag & TAG_Q else '', + 'p' if not tag & 0xfc and tag & TAG_P else '', + ' 0x%02x' % (tag & 0xff) if tag & 0xfc else '', ' w%d' % w if w else '', ' %s' % size if size is not None else '') elif (tag & 0x7f00) == TAG_NOISE: @@ -555,6 +555,10 @@ def dbg_log(data, block_size, rev, eoff, weight, *, cksum_ = crc32c(data[j_:j_+size], cksum_) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + notes.append('q!=%x' % perturb) + # check cksum cksum__ = fromle32(data[j_:j_+4]) if cksum_ != cksum__: notes.append('cksum!=%08x' % cksum__) @@ -963,6 +967,10 @@ def main(disk, blocks=None, *, cksum__ = crc32c(data[j_:j_+size], cksum__) # found a cksum? else: + # check perturb bit + if perturb != bool(tag & TAG_Q): + break + # check cksum cksum___ = fromle32(data[j_:j_+4]) if cksum__ != cksum___: break diff --git a/scripts/dbgtag.py b/scripts/dbgtag.py index c77f9db0..33c5d2dc 100755 --- a/scripts/dbgtag.py +++ b/scripts/dbgtag.py @@ -42,8 +42,8 @@ TAG_R = 0x2000 TAG_LE = 0x0000 TAG_GT = 0x1000 TAG_CKSUM = 0x3000 -TAG_Q = 0x0000 TAG_P = 0x0001 +TAG_Q = 0x0002 TAG_NOISE = 0x3100 TAG_ECKSUM = 0x3200 @@ -196,10 +196,10 @@ def tagrepr(tag, w=None, size=None, off=None): else ' -%d' % size if size else '') elif (tag & 0x7f00) == TAG_CKSUM: - return 'cksum%s%s%s' % ( - 'p' if tag & 0xff == TAG_P - else 'q' if tag & 0xff == TAG_Q - else ' 0x%02x' % (tag & 0xff), + return 'cksum%s%s%s%s%s' % ( + 'q' if not tag & 0xfc and tag & TAG_Q else '', + 'p' if not tag & 0xfc and tag & TAG_P else '', + ' 0x%02x' % (tag & 0xff) if tag & 0xfc else '', ' w%d' % w if w else '', ' %s' % size if size is not None else '') elif (tag & 0x7f00) == TAG_NOISE: