Date: Wed, 18 Nov 2020 22:40:18 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: =?UTF-8?B?U3RlZmFuIEXDn2Vy?= <se@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r367813 - head/lib/libutil Message-ID: <CAGudoHEYEZaBTDi_wPdsAc4BkDA6cBfYgxtVw4qEATt62UUPrA@mail.gmail.com> In-Reply-To: <202011181944.0AIJiUU3003699@repo.freebsd.org> References: <202011181944.0AIJiUU3003699@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/18/20, Stefan E=C3=9Fer <se@freebsd.org> wrote: > Author: se > Date: Wed Nov 18 19:44:30 2020 > New Revision: 367813 > URL: https://svnweb.freebsd.org/changeset/base/367813 > > Log: > Add function getlocalbase() to libutil. > > This function returns the path to the local software base directory, by > default "/usr/local" (or the value of _PATH_LOCALBASE in include/paths.= h > when building the world). > > The value returned can be overridden by 2 methods: > > - the LOCALBASE environment variable (ignored by SUID programs) > - else a non-default user.localbase sysctl value > > Reviewed by: hps (earlier version) > Relnotes: yes > Differential Revision: https://reviews.freebsd.org/D27236 > > Added: > head/lib/libutil/getlocalbase.3 (contents, props changed) > head/lib/libutil/getlocalbase.c (contents, props changed) > Modified: > head/lib/libutil/Makefile > head/lib/libutil/libutil.h > > Modified: head/lib/libutil/Makefile > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/lib/libutil/Makefile Wed Nov 18 19:35:30 2020 (r367812) > +++ head/lib/libutil/Makefile Wed Nov 18 19:44:30 2020 (r367813) > @@ -12,7 +12,8 @@ PACKAGE=3D runtime > LIB=3D util > SHLIB_MAJOR=3D 9 > > -SRCS=3D _secure_path.c auth.c expand_number.c flopen.c fparseln.c gr_uti= l.c > \ > +SRCS=3D _secure_path.c auth.c expand_number.c flopen.c fparseln.c \ > + getlocalbase.c gr_util.c \ > hexdump.c humanize_number.c kinfo_getfile.c \ > kinfo_getallproc.c kinfo_getproc.c kinfo_getvmmap.c \ > kinfo_getvmobject.c kld.c \ > @@ -30,7 +31,7 @@ CFLAGS+=3D -DINET6 > > CFLAGS+=3D -I${.CURDIR} -I${SRCTOP}/lib/libc/gen/ > > -MAN+=3D expand_number.3 flopen.3 fparseln.3 hexdump.3 \ > +MAN+=3D expand_number.3 flopen.3 fparseln.3 getlocalbase.3 hexdump.3 \ > humanize_number.3 kinfo_getallproc.3 kinfo_getfile.3 \ > kinfo_getproc.3 kinfo_getvmmap.3 kinfo_getvmobject.3 kld.3 \ > login_auth.3 login_cap.3 \ > > Added: head/lib/libutil/getlocalbase.3 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/lib/libutil/getlocalbase.3 Wed Nov 18 19:44:30 2020 (r367813) > @@ -0,0 +1,99 @@ > +.\" > +.\" SPDX-License-Identifier: BSD-2-Clause-FreeBSD > +.\" > +.\" Copyright 2020 Scott Long > +.\" Copyright 2020 Stefan E=C3=9Fer > +.\" > +.\" Redistribution and use in source and binary forms, with or without > +.\" modification, are permitted provided that the following conditions > +.\" are met: > +.\" 1. Redistributions of source code must retain the above copyright > +.\" notice, this list of conditions and the following disclaimer. > +.\" 2. Redistributions in binary form must reproduce the above copyright > +.\" notice, this list of conditions and the following disclaimer in t= he > +.\" documentation and/or other materials provided with the > distribution. > +.\" > +.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' A= ND > +.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, TH= E > +.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > PURPOSE > +.\" ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE > LIABLE > +.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > CONSEQUENTIAL > +.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE > GOODS > +.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION= ) > +.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, > STRICT > +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN AN= Y > WAY > +.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY = OF > +.\" SUCH DAMAGE. > +.\" > +.\" $FreeBSD$ > +.\" > +.Dd November 18, 2020 > +.Dt GETLOCALBASE 3 > +.Os > +.Sh NAME > +.Nm getlocalbase > +.Nd "return the path to the local software directory" > +.Sh LIBRARY > +.Lb libutil > +.Sh SYNOPSIS > +.In libutil.h > +.Ft const char* > +.Fn getlocalbase "void" > +.Sh DESCRIPTION > +The > +.Fn getlocalbase > +function returns the path to the local software base directory. > +Normally this is the > +.Pa /usr/local > +directory. > +First the > +.Ev LOCALBASE > +environment variable is checked. > +If that does not exist then the > +.Va user.localbase > +sysctl is checked. > +If that also does not exist then the value of the > +.Dv _PATH_LOCALBASE > +compile-time variable is used. > +If that is undefined then the default of > +.Pa /usr/local > +is used. > +.Pp > +The value returned by the > +.Fn getlocalbase > +function shall not be modified. > +.Sh IMPLEMENTATION NOTES > +Calls to > +.Fn getlocalbase > +will perform a setugid check on the running binary before checking the > +environment. > +.Sh RETURN VALUES > +The > +.Fn getlocalbase > +function always succeeds and returns a pointer to a string, whose length > +may exceed MAXPATHLEN if it has been derived from the environment variab= le > +LOCALBASE. > +No length checks are performed on the result. > +.Sh ENVIRONMENT > +The > +.Fn getlocalbase > +library function retrieves the > +.Ev LOCALBASE > +environment variable. > +.Sh ERRORS > +The > +.Fn getlocalbase > +function always succeeds. > +.Sh SEE ALSO > +.Xr env 1 , > +.Xr src.conf 5 , > +.Xr sysctl 8 > +.Sh HISTORY > +The > +.Nm > +library function first appeared in > +.Fx 13.0 . > +.Sh AUTHORS > +This > +manual page was written by > +.An Scott Long Aq Mt scottl@FreeBSD.org and Stefan E=C3=9Fer Aq Mt > se@FreeBSD.org . > > Added: head/lib/libutil/getlocalbase.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/lib/libutil/getlocalbase.c Wed Nov 18 19:44:30 2020 (r367813) > @@ -0,0 +1,74 @@ > +/*- > + * SPDX-License-Identifier: BSD-2-Clause > + * > + * Copyright 2020 Stefan E=C3=9Fer <se@freebsd.org> > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in th= e > + * documentation and/or other materials provided with the distributio= n. > + * > + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' A= ND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE > LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOO= DS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, > STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY > WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY O= F > + * SUCH DAMAGE. > + */ > + > +#include <sys/cdefs.h> > +__FBSDID("$FreeBSD$"); > + > +#include <sys/param.h> > +#include <sys/sysctl.h> > +#include <sys/limits.h> > +#include <stdlib.h> > +#include <paths.h> > +#include <libutil.h> > +#include <unistd.h> > + > +#ifndef _PATH_LOCALBASE > +#define _PATH_LOCALBASE "/usr/local" > +#endif > + > +const char * > +getlocalbase(void) > +{ > + static const int localbase_oid[2] =3D {CTL_USER, USER_LOCALBASE}; There is no use for this to be static. > + char *tmppath; > + size_t tmplen; > + static const char *localbase =3D NULL; > + > + if (issetugid() =3D=3D 0) { > + tmppath =3D getenv("LOCALBASE"); > + if (tmppath !=3D NULL && tmppath[0] !=3D '\0') > + return (tmppath); > + } > + if (sysctl(localbase_oid, 2, NULL, &tmplen, NULL, 0) =3D=3D 0 && > + (tmppath =3D malloc(tmplen)) !=3D NULL && > + sysctl(localbase_oid, 2, tmppath, &tmplen, NULL, 0) =3D=3D 0) { Apart from the concurrency issue mentioned in the comment this is just very wasteful. Instead you can have a small local buffer, say 128 bytes and pass that to be populated. The sysctl handler than can populate that and return an error if the size is too small. I don't know if sysclt api allows it to return the set size as it is. Worst case you can just retry with a bigger malloced buffer. Once you get the result you can malloc a buffer and atomic_cmpset_rel_ptr localbase to point to it. If this fails, another thread got the result, you free your buffer and return (localbase). Also the kernel counter part completely unnecessarily comes with a static 1KB buffer to hold what typically is going to be nothing. This should also be allocated as needed. Worst case you can add a trivial lock around it. > + /* > + * Check for some other thread already having > + * set localbase - this should use atomic ops. > + * The amount of memory allocated above may leak, > + * if a parallel update in another thread is not > + * detected and the non-NULL pointer is overwritten. > + */ > + if (tmppath[0] !=3D '\0' && > + (volatile const char*)localbase =3D=3D NULL) > + localbase =3D tmppath; > + else > + free((void*)tmppath); > + return (localbase); > + } > + return (_PATH_LOCALBASE); > +} > > Modified: head/lib/libutil/libutil.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/lib/libutil/libutil.h Wed Nov 18 19:35:30 2020 (r367812) > +++ head/lib/libutil/libutil.h Wed Nov 18 19:44:30 2020 (r367813) > @@ -98,6 +98,8 @@ int flopen(const char *_path, int _flags, ...); > int flopenat(int _dirfd, const char *_path, int _flags, ...); > int forkpty(int *_amaster, char *_name, > struct termios *_termp, struct winsize *_winp); > +const char * > + getlocalbase(void); > void hexdump(const void *_ptr, int _length, const char *_hdr, int _flags= ); > int humanize_number(char *_buf, size_t _len, int64_t _number, > const char *_suffix, int _scale, int _flags); > --=20 Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEYEZaBTDi_wPdsAc4BkDA6cBfYgxtVw4qEATt62UUPrA>