Date: Sat, 25 Apr 2026 14:58:49 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Dimitry Andric <dim@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>, Alex Richardson <arichardson@FreeBSD.org> Subject: Re: git: 753a166bdeb3 - main - imgact_elf: Fix uninitialized variable use in note_procstat_auxv Message-ID: <6BCE247D-8992-4603-8CFF-F7C87E464829@freebsd.org> In-Reply-To: <673E90BE-4519-46D9-BF02-458830DCC304@FreeBSD.org> References: <69ecc4df.33991.53b0871@gitrepo.freebsd.org> <7DD33B7A-A4B7-4AF7-9EA6-CF65BF388500@freebsd.org> <673E90BE-4519-46D9-BF02-458830DCC304@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
On 25 Apr 2026, at 14:53, Dimitry Andric <dim@freebsd.org> wrote > > On 25 Apr 2026, at 15:46, Jessica Clarke <jrtc27@freebsd.org> wrote: >> >> On 25 Apr 2026, at 14:42, Dimitry Andric <dim@FreeBSD.org> wrote: >> >>> The branch main has been updated by dim: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=753a166bdeb3aeba02fd9678e7360f0929007368 >>> >>> commit 753a166bdeb3aeba02fd9678e7360f0929007368 >>> Author: Alex Richardson <arichardson@FreeBSD.org> >>> AuthorDate: 2025-09-15 06:27:12 +0000 >>> Commit: Dimitry Andric <dim@FreeBSD.org> >>> CommitDate: 2026-04-25 13:42:16 +0000 >>> >>> imgact_elf: Fix uninitialized variable use in note_procstat_auxv >>> >>> Found building with latest clang >>> >>> MFC after: 3 days >>> --- >>> sys/kern/imgact_elf.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c >>> index af0841c75549..3c5fbe4df342 100644 >>> --- a/sys/kern/imgact_elf.c >>> +++ b/sys/kern/imgact_elf.c >>> @@ -2712,13 +2712,16 @@ __elfN(note_procstat_auxv)(void *arg, struct sbuf *sb, size_t *sizep) >>> struct proc *p; >>> size_t size; >>> int structsize; >>> - >>> +#if defined(COMPAT_FREEBSD32) && __ELF_WORD_SIZE == 32 >>> + structsize = sizeof(Elf32_Auxinfo); >> >> Elf_Auxinfo *is* Elf32_Auxinfo for __ELF_WORD_SIZE == 32? >> >>> +#else >>> + structsize = sizeof(Elf_Auxinfo); >>> +#endif >>> p = arg; >>> structsize = sizeof(Elf_Auxinfo); >> >> But it’s initialised right here? So that assignment is dead? >> >>> if (sb == NULL) { >>> size = 0; >>> - sb = sbuf_new(NULL, NULL, AT_COUNT * sizeof(Elf_Auxinfo), >>> - SBUF_FIXEDLEN); >>> + sb = sbuf_new(NULL, NULL, AT_COUNT * structsize, SBUF_FIXEDLEN); >> >> These are equivalent? Though this one at least I see an argument for. >> >>> sbuf_set_drain(sb, sbuf_count_drain, &size); >>> sbuf_bcat(sb, &structsize, sizeof(structsize)); >>> PHOLD(p); >>> >> >> I really don’t understand this change at all. I think all but the final >> part should be reverted > > I've only committed this because it's part of the llvm-21-update tree, and on behalf of Alex. I think this was to suppress a bunch of warnings, and I'm happing to fix it post-import, if there are no further errors. I mean, maybe once upon a time some early form of this patch was needed, but I have no clue what warning this could possibly have been fixing, it’s so clearly not right when you look at the diff. So can we please revert this now rather than maybe at some point in the future? There’s no world in which this is sensible code to have in the tree, and if you don’t revert it, I will. If anything I would expect this to introduce *new* warnings due to the dead assignment to structsize that’s been added. Jessicahome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6BCE247D-8992-4603-8CFF-F7C87E464829>