From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 15362 invoked from network); 12 Apr 2021 02:47:40 -0000 Received: from 1ess.inri.net (216.126.196.35) by inbox.vuxu.org with ESMTPUTF8; 12 Apr 2021 02:47:40 -0000 Received: from mail.9lab.org ([168.119.8.41]) by 1ess; Sun Apr 11 21:46:31 -0400 2021 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9lab.org; s=20210803; t=1618172350; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-type: content-transfer-encoding:content-transfer-encoding:content-transfer-encoding; bh=tCkxsjOFUaTdtF6bGtQD0pcZVUAPT6sE+LRBCRHe81k=; b=g2ewC93GzxRzCOAPxRmP2grTcJRRyrTd5LmKiB3VJCymrt0UMEY4Uoz7b9uaHA/isx+TVo o4DDov8wGgX5Pd1AuLOd5UriyYPg/IDV3eI2ESMj7fV4WnNgYQuuPKSDq8GssnMF46AFQi +k3yNJeRpsmvp+9nvmYWIokC3ckgpok= Received: from term.localdomain (host-185-64-155-70.ecsnet.at [185.64.155.70]) by mail.9lab.org (OpenSMTPD) with ESMTPSA id 9ba03cb8 (TLSv1.2:ECDHE-RSA-AES256-SHA:256:NO); Sun, 11 Apr 2021 22:19:10 +0200 (CEST) Message-ID: <9F7C695CA1C76DA9023BE33F535E8C8F@9lab.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit To: 9front@9front.org CC: igor@9lab.org Date: Sun, 11 Apr 2021 22:19:09 +0200 From: igor@9lab.org MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: hypervisor extension standard-aware plugin proxy-scale database Subject: [9front] upas/Mail: fix multiple suicides (use after free, double free) in mbox.c (patch) Reply-To: 9front@9front.org Precedence: bulk Below is a patch (inline) that fixes two variants of a suicide (use after free, double free) in Mail/mbox.c on latest 9front 'Mit Fruchtgeschmack' (explanation with reproducible test instructions below) when messages are deleted/moved in bulk: diff -r 4dfbef4fa4ac sys/src/cmd/upas/Mail/mbox.c --- a/sys/src/cmd/upas/Mail/mbox.c Sat Apr 03 19:32:47 2021 +0200 +++ b/sys/src/cmd/upas/Mail/mbox.c Fri Apr 09 02:54:10 2021 +0200 @@ -675,10 +675,11 @@ static void mbflush(char **, int) { - int i, j, ln, fd; + int i, j, ln, fd, nmesg; char *path; Mesg *m, *p; + nmesg = mbox.nmesg; path = estrjoin(maildir, "/ctl", nil); fd = open(path, OWRITE); free(path); @@ -708,11 +709,13 @@ mbredraw(m->child[j], 1, 1); } - for(i = 0, j = 0; i < mbox.nmesg; i++){ + for(i = 0, j = 0; i < nmesg; i++){ m = mbox.mesg[i]; - if((m->state & Szap) != 0) + if((m->state & Szap) != 0){ mesgfree(m); - else + mbox.mesg[i] = nil; + mbox.nmesg--; + }else mbox.mesg[j++] = m; } To reproduce a suicude create a dummy IMAP folder, copy some emails to it, and then select all emails and middle click 'Delmesg' followed by middle clicking 'Put'. Here is how to create a dummy IMAP folder 'Einhorn' and copy 50 mails from your Inbox to it without leaving the comfort of 9front (replace mail.9lab.org with your IMAP server): term% tlsclient tcp!mail.9lab.org!imaps a login YOURUSER YOURPASSWORD b create Einhorn c select Inbox d copy 1:50 Einhorn e logout Now you can open the 'Einhorn' folder (replace mail.9lab.org with your URL and igor with your user name): term% echo open /imaps/mail.9lab.org/igor/Einhorn Einhorn >/mail/fs/ctl Finally, select and middle click 'Mail Einhorn' in acme. Now you can try to remove all mails from the 'Einhorn' folder by selecting all messages and middle clicking 'Delmesg' followed by 'Put'. There is a very high chance upas/Mail will commit suicide like this: term% broke echo kill>/proc/1136/ctl # Mail term% acid 1136 /proc/1136/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: lstk() strcmp(s2=0x463e88)+0x12 /sys/src/libc/port/strcmp.c:10 mesgmatch(m=0x412b00,name=0x463e88,digest=0x412c80)+0x24 /sys/src/cmd/upas/Mail/mesg.c:483 mesglookup(digest=0x412c80)+0x56 /sys/src/cmd/upas/Mail/mesg.c:496 buf=0x2f33 i=0x2f3300000001 delete(digest=0x412c80)+0x13 /sys/src/cmd/upas/Mail/mbox.c:336 changemesg(pm=0x4236b8)+0x181 /sys/src/cmd/upas/Mail/mbox.c:838 digest=0x412c80 action=0x412b00 r=0x40fe00 mbmain(cmd=0x40fc80)+0x240 /sys/src/cmd/upas/Mail/mbox.c:966 a=0x40fda0 ev=0x40ee90 psee=0x4236b8 pshow=0xfefefefefefefefe psend=0xfefefefefefefefe launcheramd64(arg=0x40fc80,f=0x2023d5)+0x10 /sys/src/libthread/amd64.c:11 With the above commands to copy mails between folders you can create more variations of deleting messages in bulk from the 'Einhorn' test folder without breaking a sweat. Chances are high you will run into another suicide like this one: panic: D2B called on non-block 40f9e8 (double-free?) Mail 1967: suicide: sys: trap: fault read addr=0x0 pc=0x20fbdd ... term% broke echo kill>/proc/1967/ctl # Mail term% acid 1967 /proc/1967/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: lstk() abort()+0x0 /sys/src/libc/9sys/abort.c:6 ppanic(fmt=0x4055b1)+0x14b /sys/src/libc/port/malloc.c:166 pv=0x407068 msg=0x406f50 v=0x46a810 n=0x46a8100000002d D2B(v=0x40f168)+0x6a /sys/src/libc/port/pool.c:926 a=0x40f160 poolfreel(v=0x40f168,p=0x4001d8)+0x23 /sys/src/libc/port/pool.c:1153 ab=0x407068 poolfree(p=0x4001d8,v=0x40f168)+0x3e /sys/src/libc/port/pool.c:1287 free()+0x26 /sys/src/libc/port/malloc.c:250 mesgclear(m=0x47ed80)+0x52 /sys/src/cmd/upas/Mail/mesg.c:26 i=0x202ce000000001 mesgfree(m=0x47ed80)+0x19 /sys/src/cmd/upas/Mail/mesg.c:47 mbflush()+0x217 /sys/src/cmd/upas/Mail/mbox.c:714 path=0x47fd00 fd=0x10 i=0x20231400000004 p=0x0 m=0x47ed80 j=0x400000000 doevent(ev=0x494040)+0x11f /sys/src/cmd/upas/Mail/mbox.c:913 f=0x494058 nf=0x49405800000001 p=0x406b98 mbmain(cmd=0x40fc80)+0x205 /sys/src/cmd/upas/Mail/mbox.c:962 a=0x40fda0 ev=0x494040 psee=0x494e60 pshow=0xfefefefefefefefe psend=0xfefefefefefefefe launcheramd64(arg=0x40fc80,f=0x2023d5)+0x10 /sys/src/libthread/amd64.c:11 The first suicide is caused by use after free, the second suicude is caused by a double free. After reviewing the code paths outlined in the stack trace as well as some surrounding logic, I think the problem is due to not decrementing 'mbox.nmesg' when messages are freed. Hence logic that iterates over already deleted messages might examine already deleted/freed emails. A quick grep in cmd/upas/Mail confirms this hypothesis: term% g nmesg mail.h:134: int nmesg; mail.h:137: Mesg *openmesg; mesg.c:383: m->qnext = mbox.openmesg; mesg.c:384: mbox.openmesg = m; mesg.c:422: for(pm = &mbox.openmesg; *pm != nil; pm = &(*pm)->qnext) mesg.c:495: for(i = 0; i < mbox.nmesg; i++) mbox.c:154: for(i = 0; i < mbox.nmesg; i++){ mbox.c:167: assert(n + o <= mbox.nmesg); mbox.c:199: for(i = 0; i < mbox.nmesg; i++) mbox.c:259: if(mbox.nmesg == mbox.mesgsz){ mbox.c:266: idx = mbox.nmesg; mbox.c:267: memmove(&mbox.mesg[idx + 1], &mbox.mesg[idx], (mbox.nmesg - idx)*sizeof(Mesg*)); mbox.c:269: mbox.nmesg++; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mbox.c:274: if(mbox.hashsz <= 2*(mbox.nmesg + mbox.ndead)){ mbox.c:277: if(mbox.hashsz <= 2*mbox.nmesg) mbox.c:439: qsort(mbox.mesg, mbox.nmesg, sizeof(Mesg*), cmpmesg); mbox.c:687: for(i = 0; i < mbox.nmesg; i++){ mbox.c:711: for(i = 0, j = 0; i < mbox.nmesg; i++){ mbox.c:749: for(i = 0; i < mbox.nmesg; i++){ mbox.c:771: for(m = mbox.openmesg; m != nil; m = m->qnext) mbox.c:789: for(i = idx; i < mbox.nmesg; i++) We only increment 'mbox.nmesg' (see highlighted line). There does not seem to be a place where it is decremented. Since 'mbox.nmesg' is not decremented after freeing/deleting emails, any for loop like this one: for(i = 0; i < mbox.nmesg; i++) if(cmpmesg(&mbox.mesg[i], &m) >= 0) break; executed after freeing/deleting emails, may access already freed data. The fix proposed in the above patch decrements 'mbox.nmesg' when deleting emails to avoid use after free suicides. I have been running with this fix for a while and have not observed a suicide since. I delete/free mails in bulk a lot after copying them from my IMAP Inbox folder to other IMAP folders. The way to move emails in IMAP is to copy and then delete. To copy mails I use some hacked up rc scripts that can be middle clicked in acme to automate the process. Danke für das neueste Release, ich mag Fruchtgeschmack ☺ Cheers, Igor PS.: This is a second improved attempt to send this patch. The email I sent last week has been marked as SPAM (see /n/lists/9front/1617575697.00).