Github/littlefs
1
Fork 0
Code Actions Activity

Tweaked how grm encoding/decoding work

Browse Source 
We never encode/decode the grm to/from disk and we always know the
buffer size statically.

Even when we calculate the size for the grm tag, we ignore the encoded
size and optimistically scan for the number of trailing zeros, giving us
a potentially smaller gdelta.

This change drops the encoded length completely in grm encoding/decoding
functions, assuming all related buffers are statically sized and padded
with zeros.

This also means you can't forget to zero the buffer when encoding, which
was already overlooked several times, leading to internal garbage on
disk. So that's nice.
This commit is contained in:
Christopher Haster
2023-07-28 16:28:16 -05:00
parent e08ff99d50
commit 5f0161712c
1 changed files with 27 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+27 -38
View File
@@ -1610,9 +1610,11 @@ static inline void lfsr_grm_poprm(lfsr_grm_t *grm) {
grm->rms[1].mid = LFSR_MID_RM; grm->rms[1].mid = LFSR_MID_RM;
} }
static lfs_ssize_t lfsr_grm_todisk(lfs_t *lfs, const lfsr_grm_t *grm, static int lfsr_grm_todisk(lfs_t *lfs, const lfsr_grm_t *grm,
uint8_t buffer[static LFSR_GRM_DSIZE]) { uint8_t buffer[static LFSR_GRM_DSIZE]) {
(void)lfs; (void)lfs;
// make sure to zero so we don't leak any info
memset(buffer, 0, LFSR_GRM_DSIZE);
// first encode the number of grms, this can be 0, 1, or 2 and may // first encode the number of grms, this can be 0, 1, or 2 and may
// be extended to a general purpose leb128 type field in the future // be extended to a general purpose leb128 type field in the future
@@ -1641,21 +1643,18 @@ static lfs_ssize_t lfsr_grm_todisk(lfs_t *lfs, const lfsr_grm_t *grm,
d += d_; d += d_;
} }
return d; return 0;
} }
// needed in lfsr_grm_fromdisk // needed in lfsr_grm_fromdisk
static inline int lfsr_mtree_isinlined(lfs_t *lfs); static inline int lfsr_mtree_isinlined(lfs_t *lfs);
static lfs_ssize_t lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm, static int lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm,
lfsr_data_t data) { uint8_t buffer[static LFSR_GRM_DSIZE]) {
// get the count from the first byte
lfs_ssize_t d = 0; lfs_ssize_t d = 0;
uint8_t count = 0; uint8_t count = buffer[0];
lfs_ssize_t d_ = lfsr_data_read(lfs, data, d, &count, 1); d += 1;
if (d_ < 0) {
return d_;
}
d += d_;
// clear first // clear first
grm->rms[0].mid = LFSR_MID_RM; grm->rms[0].mid = LFSR_MID_RM;
@@ -1664,7 +1663,7 @@ static lfs_ssize_t lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm,
LFS_ASSERT(count <= 2); LFS_ASSERT(count <= 2);
for (uint8_t i = 0; i < count; i++) { for (uint8_t i = 0; i < count; i++) {
lfs_size_t mid; lfs_size_t mid;
d_ = lfsr_data_readleb128(lfs, data, d, &mid); lfs_ssize_t d_ = lfs_fromleb128(&mid, &buffer[d], LFSR_GRM_DSIZE-d);
if (d_ < 0) { if (d_ < 0) {
return d_; return d_;
} }
@@ -1673,7 +1672,7 @@ static lfs_ssize_t lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm,
LFS_ASSERT(mid < 0x7fffffff); LFS_ASSERT(mid < 0x7fffffff);
lfs_size_t rid; lfs_size_t rid;
d_ = lfsr_data_readleb128(lfs, data, d, &rid); d_ = lfs_fromleb128(&rid, &buffer[d], LFSR_GRM_DSIZE-d);
if (d_ < 0) { if (d_ < 0) {
return d_; return d_;
} }
@@ -1694,7 +1693,7 @@ static lfs_ssize_t lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm,
grm->rms[i].rid = rid; grm->rms[i].rid = rid;
} }
return d; return 0;
} }
static inline bool lfsr_grm_iszero(const uint8_t gdelta[LFSR_GRM_DSIZE]) { static inline bool lfsr_grm_iszero(const uint8_t gdelta[LFSR_GRM_DSIZE]) {
@@ -5837,16 +5836,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
for (lfs_size_t i = 0; i < attr_count; i++) { for (lfs_size_t i = 0; i < attr_count; i++) {
if (attrs[i].tag == LFSR_TAG_GRM) { if (attrs[i].tag == LFSR_TAG_GRM) {
// encode to disk // encode to disk
// TODO move this into lfsr_grm_todisk? int err = lfsr_grm_todisk(lfs, attrs[i].d.grm, lfs->dgrm);
// make sure to zero first so we don't leak anything if (err) {
memset(lfs->dgrm, 0, LFSR_GRM_DSIZE); return err;
lfs_ssize_t d = lfsr_grm_todisk(lfs, attrs[i].d.grm, lfs->dgrm);
if (d < 0) {
return d;
} }
// xor with our current gstate to find our initial gdelta // xor with our current gstate to find our initial gdelta
int err = lfsr_grm_xor(lfs, lfs->dgrm, LFSR_DATA_BUF( err = lfsr_grm_xor(lfs, lfs->dgrm, LFSR_DATA_BUF(
lfs->pgrm, LFSR_GRM_DSIZE)); lfs->pgrm, LFSR_GRM_DSIZE));
if (err) { if (err) {
return err; return err;
@@ -6038,14 +6034,12 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
// //
lfsr_grm_t *grm = attrs[i].d.grm; lfsr_grm_t *grm = attrs[i].d.grm;
uint8_t buf[LFSR_GRM_DSIZE]; uint8_t buf[LFSR_GRM_DSIZE];
// make sure to zero so we don't end up with trailing garbage int err = lfsr_grm_todisk(lfs, grm, buf);
memset(buf, 0, LFSR_GRM_DSIZE); if (err) {
lfs_ssize_t d = lfsr_grm_todisk(lfs, grm, buf); return err;
if (d < 0) {
return d;
} }
int err = lfsr_grm_xor(lfs, lfs->dgrm, err = lfsr_grm_xor(lfs, lfs->dgrm,
LFSR_DATA_BUF(buf, LFSR_GRM_DSIZE)); LFSR_DATA_BUF(buf, LFSR_GRM_DSIZE));
if (err) { if (err) {
return err; return err;
@@ -6083,11 +6077,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
} }
// xor our fix into our gdelta // xor our fix into our gdelta
// make sure to zero so we don't end up with trailing garbage err = lfsr_grm_todisk(lfs, grm, buf);
memset(buf, 0, LFSR_GRM_DSIZE); if (err) {
d = lfsr_grm_todisk(lfs, grm, buf); return err;
if (d < 0) {
return d;
} }
err = lfsr_grm_xor(lfs, lfs->dgrm, err = lfsr_grm_xor(lfs, lfs->dgrm,
@@ -6218,10 +6210,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
lfs->grm = *attrs[i].d.grm; lfs->grm = *attrs[i].d.grm;
// keep track of the exact encoding on-disk // keep track of the exact encoding on-disk
memset(lfs->pgrm, 0, LFSR_GRM_DSIZE); int err = lfsr_grm_todisk(lfs, &lfs->grm, lfs->pgrm);
lfs_ssize_t d = lfsr_grm_todisk(lfs, &lfs->grm, lfs->pgrm); if (err) {
if (d < 0) { return err;
return d;
} }
} }
} }
@@ -7157,9 +7148,7 @@ static int lfsr_mountinited(lfs_t *lfs) {
memcpy(lfs->pgrm, lfs->dgrm, LFSR_GRM_DSIZE); memcpy(lfs->pgrm, lfs->dgrm, LFSR_GRM_DSIZE);
// decode grm so we can report any removed files as missing // decode grm so we can report any removed files as missing
// TODO wait, should mdir_commit update lfs->grm as well? lfs_ssize_t d = lfsr_grm_fromdisk(lfs, &lfs->grm, lfs->pgrm);
lfs_ssize_t d = lfsr_grm_fromdisk(lfs, &lfs->grm,
LFSR_DATA_BUF(lfs->pgrm, LFSR_GRM_DSIZE));
if (d < 0) { if (d < 0) {
return d; return d;
} }