Date: Tue, 6 Jan 2026 08:37:12 -0600 From: John Baldwin <jhb@FreeBSD.org> To: =?UTF-8?Q?Dag-Erling_Sm=C3=B8rg_rav?= <des@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 27894e20f140 - main - libgeom: Fix segfault in 32-on-64 case Message-ID: <cce035dc-ca9b-4dc0-81f7-22b92da52217@FreeBSD.org> In-Reply-To: <6958dd10.b4b9.2aebecda@gitrepo.freebsd.org>
index | next in thread | previous in thread | raw e-mail
On 1/3/26 04:10, Dag-Erling Smørg rav wrote: > The branch main has been updated by des: > > URL: https://cgit.FreeBSD.org/src/commit/?id=27894e20f140ee2729c14b589035870c8185b87d > > commit 27894e20f140ee2729c14b589035870c8185b87d > Author: Dag-Erling Smørgrav <des@FreeBSD.org> > AuthorDate: 2026-01-03 09:09:51 +0000 > Commit: Dag-Erling Smørgrav <des@FreeBSD.org> > CommitDate: 2026-01-03 09:10:23 +0000 > > libgeom: Fix segfault in 32-on-64 case > > We were using strtoul() to parse object identifiers, which are kernel > pointers. This works fine as long as the kernel and userland match, > but in a 32-bit libgeom on a 64-bit kernel this will return ULONG_MAX > for all objects, resulting in memory corruption when we later pick the > wrong object while resolving consumer-producer references. > > MFC after: 1 week > PR: 292127 > Reviewed by: imp > Differential Revision: https://reviews.freebsd.org/D54452 > --- > lib/libgeom/geom_xml2tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/libgeom/geom_xml2tree.c b/lib/libgeom/geom_xml2tree.c > index 2d2c43e29e77..161425d9fadf 100644 > --- a/lib/libgeom/geom_xml2tree.c > +++ b/lib/libgeom/geom_xml2tree.c > @@ -76,10 +76,10 @@ StartElement(void *userData, const char *name, const char **attr) > ref = NULL; > for (i = 0; attr[i] != NULL; i += 2) { > if (!strcmp(attr[i], "id")) { > - id = (void *)strtoul(attr[i + 1], NULL, 0); > + id = (void *)strtoumax(attr[i + 1], NULL, 0); > mt->nident++; > } else if (!strcmp(attr[i], "ref")) { > - ref = (void *)strtoul(attr[i + 1], NULL, 0); > + ref = (void *)strtoumax(attr[i + 1], NULL, 0); Should we perhaps not use pointers to hold the cookies? This is going to truncate in the lib32 case which will probably still work in practice as the low 32 bits of kernel object addresses are probably unique, but isn't foolproof. Perhaps the cookie values should be stored as either kvaddr_t values, or uintmax_t? -- John Baldwinhome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?cce035dc-ca9b-4dc0-81f7-22b92da52217>