Date: Sat, 25 Apr 2026 15:00:29 +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: <250813FE-84A4-4C61-8072-6CA106DADA41@freebsd.org> In-Reply-To: <6BCE247D-8992-4603-8CFF-F7C87E464829@freebsd.org> References: <69ecc4df.33991.53b0871@gitrepo.freebsd.org> <7DD33B7A-A4B7-4AF7-9EA6-CF65BF388500@freebsd.org> <673E90BE-4519-46D9-BF02-458830DCC304@FreeBSD.org> <6BCE247D-8992-4603-8CFF-F7C87E464829@freebsd.org>
index | next in thread | previous in thread | raw e-mail
On 25 Apr 2026, at 14:58, Jessica Clarke <jrtc27@freebsd.org> wrote: > > 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. Probably 8a68c2509c00 ("imgact_elf: Unconditionally initialize a variable in a note handler”) is what has made this diff become nonsense and unnecessary. Jessicahome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?250813FE-84A4-4C61-8072-6CA106DADA41>