Skip to content

fix: prevent panic in writePage when partition runs out of pages at a namespace boundary - #61

Open
jgangemi wants to merge 1 commit into
tinygo-org:mainfrom
jgangemi:jae/nvs-writepage-insufficient-pages
Open

fix: prevent panic in writePage when partition runs out of pages at a namespace boundary#61
jgangemi wants to merge 1 commit into
tinygo-org:mainfrom
jgangemi:jae/nvs-writepage-insufficient-pages

Conversation

@jgangemi

@jgangemi jgangemi commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

The bug

writePage slices the partition buffer at pageOffset:pageOffset+PageSize before checking pageNum against totalPages:

func writePage(partition *[]byte, startPageNum int, seqNum uint32, entries []*entry, totalPages int) (int, error) {
	pageNum := startPageNum
	pageOffset := pageNum * PageSize
	page := (*partition)[pageOffset : pageOffset+PageSize]

The mid-item overflow path (startNewPage, called once a page is already partially filled) already guards against running out of pages and returns a clean not enough pages error. This initial slice, taken before any entry has been placed on the page, has no equivalent guard.

writePage gives every namespace its own fresh page — it never packs a new namespace's entries into a previous namespace's leftover slots — so GenerateNVS needs at least as many pages as there are distinct namespaces. When that requirement already exactly exhausts the partition, starting the next namespace calls writePage with startPageNum == totalPages, and the initial slice reads straight past the end of the partition.

On a stock 6-page (0x6000) NVS partition this reproduces as:

panic: runtime error: slice bounds out of range [:28672] with capacity 24576

(6 * 4096 = 24576 is the partition's actual capacity; the slice tries to read to 7 * 4096 = 28672.)

The fix

Add the same totalPages guard to the initial slice that startNewPage already has, so this case returns the existing not enough pages error instead of panicking:

if pageNum >= totalPages {
	return 0, fmt.Errorf("not enough pages: need at least %d pages", pageNum+1)
}

Repro / regression test

Added TestGenerateNVSInsufficientPagesAtNamespaceBoundaryReturnsErrorNotPanic in pkg/nvs/nvs_test.go: 6 single-key namespaces exactly fill a 6-page partition (one page per namespace), then a 7th brand-new namespace is added, pushing the running page index to exactly 6 == totalPages before any entry is placed. Before the fix this panics; after, it returns a clean error.

Mutation-checked locally: reverting just the added guard reproduces the exact slice bounds out of range [:28672] with capacity 24576 panic on this new test; restoring the guard is byte-identical to this diff and the suite is green again.

How this was found

Found via a downstream tool (esp_nvs_set/esp_nvs_delete operations) hitting this panic against a real ESP32 with a 0x6000 NVS partition when writing a namespace that pushed the generated image exactly to the partition's page limit.

… namespace boundary

writePage sliced the partition buffer at pageOffset:pageOffset+PageSize
before checking pageNum against totalPages. The mid-item overflow path
(startNewPage) already guarded against running out of pages, but this
initial slice did not, so a namespace boundary landing exactly at
startPageNum == totalPages read past the end of the partition slice
and panicked (slice bounds out of range) instead of returning the
same not enough pages error the mid-item path already produces.

Add the missing totalPages check before the initial slice.
jgangemi added a commit to dangernoodle-io/pogopin that referenced this pull request Aug 9, 2026
…fix (BR-97)

esp_nvs_set and esp_nvs_delete panic ("slice bounds out of range") when regenerating an NVS partition that needs more pages than the caller's partition size - a bug in espflasher's nvs.writePage(). Upstream fix is submitted as tinygo-org/espflasher#61 (jgangemi/espflasher fork, branch jae/nvs-writepage-insufficient-pages, commit d4c80b5) but not yet merged or tagged.

- add a TEMPORARY go.mod replace against the public jgangemi/espflasher fork module (resolved pseudo-version v0.0.0-20260809015630-d4c80b561746), not a local filesystem path, so it resolves on any machine including CI runners
- two regression tests already covered the panic; both now pass against the fork's fix

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
jgangemi added a commit to dangernoodle-io/pogopin that referenced this pull request Aug 9, 2026
…fix (BR-97) (#75)

esp_nvs_set and esp_nvs_delete panic ("slice bounds out of range") when regenerating an NVS partition that needs more pages than the caller's partition size - a bug in espflasher's nvs.writePage(). Upstream fix is submitted as tinygo-org/espflasher#61 (jgangemi/espflasher fork, branch jae/nvs-writepage-insufficient-pages, commit d4c80b5) but not yet merged or tagged.

- add a TEMPORARY go.mod replace against the public jgangemi/espflasher fork module (resolved pseudo-version v0.0.0-20260809015630-d4c80b561746), not a local filesystem path, so it resolves on any machine including CI runners
- two regression tests already covered the panic; both now pass against the fork's fix

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant