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=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 31584 invoked from network); 28 Feb 2021 16:48:55 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 28 Feb 2021 16:48:55 -0000 Received: (qmail 5259 invoked by uid 550); 28 Feb 2021 16:48:53 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 5241 invoked from network); 28 Feb 2021 16:48:52 -0000 Date: Sun, 28 Feb 2021 11:48:40 -0500 From: Rich Felker To: Benedict Cc: musl Message-ID: <20210228164839.GE32655@brightrain.aerifal.cx> References: <32f125fa.2032.177e95996dd.Coremail.hymeng90@126.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <32f125fa.2032.177e95996dd.Coremail.hymeng90@126.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] realloc buffer overflow issue On Sun, Feb 28, 2021 at 11:54:58PM +0800, Benedict wrote: > I found an memory leak issue in my multi-thread, which had fixed issue by the following patch > > > > https://git.musl-libc.org/cgit/musl/commit/src/malloc?id=3e16313f8fe2ed143ae0267fd79d63014c24779f > > > > > > > about this patch, I think it involved a bug for realloc function: > when the request size 'n' (the second paramter passed to realloc) > lesser than the original memory size 'n0' (the memory block that > first paramter of realloc pointed to) and the next chunk of 'self ' > is in used state, the realloc should call trim(self, n) to split > current chunk rather than call malloc, that will cause memcpy buffer > overflow due to 'n < n0', and it will cause next malloc crash... > > > after fix, I think it should be like: > lock(mal.split_merge_lock); > > > size_t nsize = next->csize & C_INUSE ? 0 : CHUNK_SIZE(next); > if (nsize)) { > int i = bin_index(nsize); > lock_bin(i); > if (!(next->csize & C_INUSE)) { > unbin(next, i); > next = NEXT_CHUNK(next); > self->csize = next->psize = n0+nsize | C_INUSE; > } > unlock_bin(i); > } > > if(CHUNK_SIZE(self) >= n) { > trim(self, n); > unlock(mal.split_merge_lock); > return CHUNK_TO_MEM(self); > } > unlock(mal.split_merge_lock); > > > copy_realloc: > /* As a last resort, allocate a new chunk and copy to it. */ > new = malloc(n-OVERHEAD); > if (!new) return 0; > copy_free_ret: > memcpy(new, p, n0-OVERHEAD); > free(CHUNK_TO_MEM(self)); > return new; I haven't looked in detail yet but isn't this the same bug fixed a week after it was introduced with commits cb5babdc8d624a3e3e7bea0b4e28a677a2f2fc46 and fca7428c096066482d8c3f52450810288e27515c ? Rich