From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E82671FC101; Thu, 14 May 2026 23:13:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778800437; cv=none; b=C52zXOeynToZkrZyv4kJbCCJfGnFQ1Lf56JfjBfUVrPr+DhoV66/jLeiFItrN1NXHc9ALxm0k31mRDc7WyMn8EY+Dac25HRTvUg2mdMBaXt9izg68hqGt7Y0xsBz5e7o0Q9cUuSgXLY3M4+riKn0Cx0IZTCG+MgHSKT7YdAPrFo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778800437; c=relaxed/simple; bh=QyMVF0uvG9KUHwhsL0Eq8/ietjFflpcbXOwE8C7wqCU=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=jwl1o1KzmfPr7hRfAgZBTNSyxTDD5YAkGjETjJDriCl4ZCq8hh2voYDbs0VTShd8qyhvmDPFA+j0NWYIs+WMO7oNuTS4MCb7radzqxLi2hebHrdJaDsixzspgpeT8fq0pLpW2f3VjaIbf//C7xtGWZl13qarm8wvPWCRc1jpB8M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=mfuh03Ng; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="mfuh03Ng" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DD8D7C2BCB3; Thu, 14 May 2026 23:13:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778800436; bh=QyMVF0uvG9KUHwhsL0Eq8/ietjFflpcbXOwE8C7wqCU=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=mfuh03NgX/Go16wsBTOGvc06SxamGu7YJj7eMNaLPoAlItjCAJErKtt9cEK1seu+J YC2cofJLMSf9MvpZgJQfBE4tOaN2mdGW4z3X/krDR3H7+L7CO+OuZiWuJQFjtV4oAF 8EELHB5yDw1FmXuGJi8DaPIe/PeKmffNGSfEKvpTZAFO+bIvY+eAISneBrCQjigOsB rkEyNgHGlNnEJUoSh8dxUlwKJRGkC91aHA7w+NgcV/QYm137VuntrZoC6bDol8FmIg TzzF4RANjnVkfxp8iqjem/Tc8dWvdQ2EUfTEOoFxbWYJUbpNqEZnM9Wv+SZQMufngu 6PoUlFE+MeYHw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v3 12/21] objtool: Refactor elf_add_data() to use a growable data buffer Reply-To: sashiko-reviews@lists.linux.dev To: x86@kernel.org Cc: linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, "Will Deacon" , "Peter Zijlstra" , linux-kernel@vger.kernel.org, "Catalin Marinas" , "Joe Lawrence" , "Song Liu" , "Mark Rutland" , "Petr Mladek" , "Miroslav Benes" In-Reply-To: References: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Thu, 14 May 2026 23:13:55 +0000 Message-Id: <20260514231355.DD8D7C2BCB3@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [Medium] An integer overflow in `elf_add_data()` due to an uncontrolled `= sh_addralign` value allows an out-of-bounds heap write. - [Low] In `elf_add_data`, a failed `calloc` for the new data buffer overwr= ites `sec->data->d_buf` with NULL and leaks the original buffer. -- commit 02f18d399bba6e3d751363b4e85f507e3f288e34 Author: Josh Poimboeuf objtool: Refactor elf_add_data() to use a growable data buffer Instead of calling elf_newdata() for each new piece of data with its own separate buffer, keep it all in the same growable buffer so the section's entire data can be accessed if needed. > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c > --- a/tools/objtool/elf.c > +++ b/tools/objtool/elf.c [ ... ] > @@ -1409,30 +1406,55 @@ void *elf_add_data(struct elf *elf, struct sectio= n *sec, const void *data, size_ > return NULL; > } > =20 > - sec->data =3D elf_newdata(s); > if (!sec->data) { [ ... ] > + sec->data->d_align =3D sec->sh.sh_addralign; > } > =20 > - sec->data->d_buf =3D calloc(1, size); > - if (!sec->data->d_buf) { > - ERROR_GLIBC("calloc"); > - return NULL; > + size_old =3D sec->data->d_size; > + offset =3D ALIGN(size_old, sec->sh.sh_addralign); > + size_new =3D offset + size; Could this ALIGN calculation overflow if a malformed ELF file provides an extremely large sh_addralign value? If offset wraps around, it seems size_new might also wrap, potentially caus= ing roundup_pow_of_two to return a small allocation size. Could this lead to an out-of-bounds write during the subsequent memcpy further down in this funct= ion? > - if (data) > - memcpy(sec->data->d_buf, data, size); > + if (!sec->data_overallocated) > + alloc_size_old =3D size_old; > + else > + alloc_size_old =3D max(64UL, roundup_pow_of_two(size_old ? : 1)); > =20 > - sec->data->d_size =3D size; > - sec->data->d_align =3D sec->sh.sh_addralign; > + alloc_size_new =3D max(64UL, roundup_pow_of_two(size_new ? : 1)); > =20 > - offset =3D ALIGN(sec_size(sec), sec->sh.sh_addralign); > - sec->sh.sh_size =3D offset + size; > + if (alloc_size_new > alloc_size_old) { > + void *orig_buf =3D sec->data->d_buf; > + > + sec->data->d_buf =3D calloc(1, alloc_size_new); > + if (!sec->data->d_buf) { > + ERROR_GLIBC("calloc"); > + return NULL; > + } If calloc fails here, does assigning its result directly to sec->data->d_buf clobber the valid pointer to orig_buf? It appears orig_buf is never freed on this error path, which might leak the original buffer if it was previously owned by objtool. Would it be safer to keep the original buffer pointer intact on allocation failure? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1778642120.gi= t.jpoimboe@kernel.org?part=3D12