From owner-svn-src-all@freebsd.org Wed Nov 18 21:40:22 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 2D544472F06; Wed, 18 Nov 2020 21:40:22 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cbx7B0PWKz4h7p; Wed, 18 Nov 2020 21:40:21 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x32b.google.com with SMTP id s13so4106531wmh.4; Wed, 18 Nov 2020 13:40:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=6FoiPQVafvvEdoMEiwZTYAY5/6nHgYTJJWVZ5tK7aUs=; b=WQnWYUp/s/c3l7lkERYX8CizTIOEiU5gyq7EF0Ia0DGdKzM5ntUomGR1UEb4wu8VMK 2QI3ma2Ku1uC+qFig/4W34Qomn/72dn+lZdDjMf6Qz076iAFpQde+Gfk48qgVX50RoD9 9L8nkBTHnYpMbj1eaubQ+kLMtV+k6dY+1B+mbScqplMnGnew+Un8eLAcCUe3UjNpiRxK P5NRgElKoVCh/H+coJRav/WUwBpqPZd1EyGC3Yuc5+CXs9X+Z9pI1fm4tjjXb5XoXnwt dPX9S5C6mw/nbRuGwgJYoutyfL+E2USkZchoBWwpQUSYyu35zCGrYYeNJlQS1+Bez800 uTBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=6FoiPQVafvvEdoMEiwZTYAY5/6nHgYTJJWVZ5tK7aUs=; b=uWou1AC1XRlMs0zTSTjw7q1Bw7mgkYKsuLuVJfSJI9FUYlAscgXfwU2jqjnZvzYUCi pNbt+U8MQKRWk5Sk0/GDdTpv3hu1cMJdi+D6xp8deENPV+oIGW5/j30sUzJciBMAGc2D Kgfllb7addEL69tbH9wRaKPmBZLKWdxc8lFk/ZrDxEqo8T2wVrzXCclugAgZAQYx6JQd 8lNUPVhovLGA9ToM8rrJNN8Gm/EUBEpbS35LiAEPANIg5SL3dtelHMNWIhUkal47DlPN cviRbA7a7BaFlKTwsYbHKsLA8n8NLzfJC31zuNNvIswOKcwbPnT2QP+2abZPlKtoK6jH GTqw== X-Gm-Message-State: AOAM530pkP6uXNWOgpS9DE2fPSeD56A5KkwqK/alM6WsbeF6d1oIMjJG GeZ+Do9A+sUF476sYgpqajAZad6jPnrf9vfBcGgPVxs6HbE= X-Google-Smtp-Source: ABdhPJy3iwixSjNlNxW0u5HkplMwTSpA8rIHRA38YNgSbNW/G0MP/jMS6EsH1AF0JLfKOeZXvvsvHDqI5EzLcSbkkD8= X-Received: by 2002:a05:600c:2110:: with SMTP id u16mr1104677wml.178.1605735619562; Wed, 18 Nov 2020 13:40:19 -0800 (PST) MIME-Version: 1.0 Received: by 2002:adf:dec7:0:0:0:0:0 with HTTP; Wed, 18 Nov 2020 13:40:18 -0800 (PST) In-Reply-To: <202011181944.0AIJiUU3003699@repo.freebsd.org> References: <202011181944.0AIJiUU3003699@repo.freebsd.org> From: Mateusz Guzik Date: Wed, 18 Nov 2020 22:40:18 +0100 Message-ID: Subject: Re: svn commit: r367813 - head/lib/libutil To: =?UTF-8?B?U3RlZmFuIEXDn2Vy?= Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4Cbx7B0PWKz4h7p X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Nov 2020 21:40:22 -0000 On 11/18/20, Stefan E=C3=9Fer 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 > + * > + * 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 > +__FBSDID("$FreeBSD$"); > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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