aerc: imap: fix SeqMap.Pop runtime error v1 APPLIED

Koni Marti: 1
 imap: fix SeqMap.Pop runtime error

 1 files changed, 3 insertions(+), 3 deletions(-)

[PATCH aerc] imap: fix SeqMap.Pop runtime error Export this patch

Fix a runtime error in the SeqMap.Pop function causing a
index-out-of-range panic:

Version: 0.18.2.r22.gfff69046 (go1.22.6 amd64 linux 2024-08-10)
Error: runtime error: index out of range [487] with length 487

goroutine 24430 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:24 +0x5e
git.sr.ht/~rjarry/aerc/lib/log.PanicHandler()
	git.sr.ht/~rjarry/aerc/lib/log/panic-logger.go:49 +0x66a
panic({0x5dbf5a688020?, 0xc002ab0d80?})
	runtime/panic.go:770 +0x132
git.sr.ht/~rjarry/aerc/worker/imap.(*SeqMap).Pop(0xc0003d4940, 0x1e8)
	git.sr.ht/~rjarry/aerc/worker/imap/seqmap.go:64 +0x17c
git.sr.ht/~rjarry/aerc/worker/imap.(*IMAPWorker).handleImapUpdate(0xc0003d4820, {0x5dbf5a6dbd00, 0xc002068708})
	git.sr.ht/~rjarry/aerc/worker/imap/worker.go:296 +0x26c
git.sr.ht/~rjarry/aerc/worker/imap.(*IMAPWorker).drainUpdates.func1()
	git.sr.ht/~rjarry/aerc/worker/imap/flags.go:29 +0x10c
created by git.sr.ht/~rjarry/aerc/worker/imap.(*IMAPWorker).drainUpdates in goroutine 52
	git.sr.ht/~rjarry/aerc/worker/imap/flags.go:21 +0x78

SeqMap.Pop uses two locks in the same function: first lock grabs the
size of the slice, second lock removes the given index in the slice.
Combine the two locks to prevent a change of the slice size before the
index can be removed.

Reported-by: Moritz Poldrack <moritz@poldrack.dev>
Signed-off-by: Koni Marti <koni.marti@gmail.com>
---

Robin Jarry <robin@jarry.cc>

a month ago

Acked-by: Robin Jarry <robin@jarry.cc>

Applied, thanks.

To git@git.sr.ht:~rjarry/aerc
   f07038bd98de..4e950b989e30  master -> master

 worker/imap/seqmap.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/worker/imap/seqmap.go b/worker/imap/seqmap.go
index 4a845865..1e64d377 100644
--- a/worker/imap/seqmap.go
+++ b/worker/imap/seqmap.go
@@ -57,13 +57,13 @@ func (s *SeqMap) Put(uid uint32) {
 // Pop removes seqnum from the SeqMap. seqnum must be a valid seqnum, ie
 // [1:size of mailbox]
 func (s *SeqMap) Pop(seqnum uint32) (uint32, bool) {
-	if int(seqnum) > s.Size() || seqnum < 1 {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	if int(seqnum) > len(s.m) || seqnum < 1 {
 		return 0, false
 	}
-	s.lock.Lock()
 	uid := s.m[seqnum-1]
 	s.m = append(s.m[:seqnum-1], s.m[seqnum:]...)
-	s.lock.Unlock()
 	return uid, true
 }
 
-- 
2.45.2

builds.sr.ht <builds@sr.ht>

a month ago

aerc/patches: SUCCESS in 1m57s

[imap: fix SeqMap.Pop runtime error][0] from [Koni Marti][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/54660
[1]: mailto:koni.marti@gmail.com

✓ #1310090 SUCCESS aerc/patches/openbsd.yml     https://builds.sr.ht/~rjarry/job/1310090
✓ #1310089 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1310089

Robin Jarry <robin@jarry.cc>

a month ago

Koni Marti <koni.marti@gmail.com> wrote: