From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K8i0W-0008RS-Sf for qemu-devel@nongnu.org; Tue, 17 Jun 2008 16:42:25 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K8i0S-0008M0-2J for qemu-devel@nongnu.org; Tue, 17 Jun 2008 16:42:20 -0400 Received: from [199.232.76.173] (port=52697 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K8i0Q-0008LZ-Tq for qemu-devel@nongnu.org; Tue, 17 Jun 2008 16:42:18 -0400 Received: from server1linux.rebelnetworks.com ([66.135.44.167]:58614) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1K8i0Q-0004Bb-9l for qemu-devel@nongnu.org; Tue, 17 Jun 2008 16:42:18 -0400 Received: from p5b1653e2.dip.t-dialin.net ([91.22.83.226] helo=phoenix2.frop.org) by server1linux.rebelnetworks.com with esmtpa (Exim 4.68) (envelope-from ) id 1K8i0A-0003ir-OD for qemu-devel@nongnu.org; Tue, 17 Jun 2008 15:42:03 -0500 From: Julian Seward Date: Tue, 17 Jun 2008 22:35:19 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200806172235.19974.jseward@acm.org> Subject: [Qemu-devel] buffer overruns in qemu-system-sparc (hw/eccmemctl.c: ecc_reset()) Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is in qemu svn trunk. Running 'qemu-system-sparc -M SS-10' (or SS-20) produces a bunch of invalid writes in hw/eccmemctl.c: ecc_reset(), as shown at the end of this message. Sometimes these are sufficiently severe to corrupt the heap and cause glibc to abort qemu. Problem does not happen with -M SS-5. The writes are caused by 290: s->regs[ECC_VCR] = 0; 291: s->regs[ECC_MFAR0] = 0x07c00000; 292: s->regs[ECC_MFAR1] = 0; In fact all the assignments to s->regs[] are very strange. regs[] is declared with 9 elements: #define ECC_NREGS 9 ... uint32_t regs[ECC_NREGS]; so valid indices should be 0 .. 8. But the constants ECC_* used to index in .. /* Register offsets */ #define ECC_MER 0 /* Memory Enable Register */ #define ECC_MDR 4 /* Memory Delay Register */ #define ECC_MFSR 8 /* Memory Fault Status Register */ #define ECC_VCR 12 /* Video Configuration Register */ #define ECC_MFAR0 16 /* Memory Fault Address Register 0 */ #define ECC_MFAR1 20 /* Memory Fault Address Register 1 */ #define ECC_DR 24 /* Diagnostic Register */ #define ECC_ECR0 28 /* Event Count Register 0 */ #define ECC_ECR1 32 /* Event Count Register 1 */ are register offsets, not indexes; AFAICS they should be divided by sizeof(uint32_t) before use. Even stranger .. at eccmemctl.c:297, having (incorrectly) filled in s->regs[], there is a loop that zeroes all but one of the entries: for (i = 1; i < ECC_NREGS; i++) s->regs[i] = 0; The following kludge removes the invalid writes, but is not elegant, and still leaves the question of why the loop zeroes out the entries afterwards. J Index: hw/eccmemctl.c =================================================================== --- hw/eccmemctl.c (revision 4744) +++ hw/eccmemctl.c (working copy) @@ -281,18 +281,18 @@ static void ecc_reset(void *opaque) { ECCState *s = opaque; - int i; + int i, four = sizeof(s->regs[0]); - s->regs[ECC_MER] &= (ECC_MER_VER | ECC_MER_IMPL); - s->regs[ECC_MER] |= ECC_MER_MRR; - s->regs[ECC_MDR] = 0x20; - s->regs[ECC_MFSR] = 0; - s->regs[ECC_VCR] = 0; - s->regs[ECC_MFAR0] = 0x07c00000; - s->regs[ECC_MFAR1] = 0; - s->regs[ECC_DR] = 0; - s->regs[ECC_ECR0] = 0; - s->regs[ECC_ECR1] = 0; + s->regs[ECC_MER / four] &= (ECC_MER_VER | ECC_MER_IMPL); + s->regs[ECC_MER / four] |= ECC_MER_MRR; + s->regs[ECC_MDR / four] = 0x20; + s->regs[ECC_MFSR / four] = 0; + s->regs[ECC_VCR / four] = 0; + s->regs[ECC_MFAR0 / four] = 0x07c00000; + s->regs[ECC_MFAR1 / four] = 0; + s->regs[ECC_DR / four] = 0; + s->regs[ECC_ECR0 / four] = 0; + s->regs[ECC_ECR1 / four] = 0; for (i = 1; i < ECC_NREGS; i++) s->regs[i] = 0; The original problem: ==21509== Invalid write of size 4 ==21509== at 0x42E056: ecc_reset (eccmemctl.c:290) ==21509== by 0x42E149: ecc_init (eccmemctl.c:323) ==21509== by 0x425E7F: sun4m_hw_init (sun4m.c:547) ==21509== by 0x40D50E: main (vl.c:8609) ==21509== Address 0x7af3c20 is 8 bytes after a block of size 48 alloc'd ==21509== at 0x4C234BB: malloc (vg_replace_malloc.c:207) ==21509== by 0x42F995: qemu_mallocz (qemu-malloc.c:44) ==21509== by 0x42E0DA: ecc_init (eccmemctl.c:306) ==21509== by 0x425E7F: sun4m_hw_init (sun4m.c:547) ==21509== by 0x40D50E: main (vl.c:8609) ==21509== ==21509== Invalid write of size 4 ==21509== at 0x42E05D: ecc_reset (eccmemctl.c:291) ==21509== by 0x42E149: ecc_init (eccmemctl.c:323) ==21509== by 0x425E7F: sun4m_hw_init (sun4m.c:547) ==21509== by 0x40D50E: main (vl.c:8609) ==21509== Address 0x7af3c30 is not stack'd, malloc'd or (recently) free'd ==21509== ==21509== Invalid write of size 4 ==21509== at 0x42E064: ecc_reset (eccmemctl.c:292) ==21509== by 0x42E149: ecc_init (eccmemctl.c:323) ==21509== by 0x425E7F: sun4m_hw_init (sun4m.c:547) ==21509== by 0x40D50E: main (vl.c:8609) ==21509== Address 0x7af3c40 is 8 bytes before a block of size 296 alloc'd ==21509== at 0x4C234BB: malloc (vg_replace_malloc.c:207) ==21509== by 0x40973E: register_savevm (vl.c:5958) ==21509== by 0x42E134: ecc_init (eccmemctl.c:321) ==21509== by 0x425E7F: sun4m_hw_init (sun4m.c:547) ==21509== by 0x40D50E: main (vl.c:8609)