Date: Wed, 26 Aug 2020 13:32:29 -0400 From: J David <j.david.lists@gmail.com> To: Brandon Bergren <bdragon@freebsd.org> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: pidfile_open() usage in "mount" Message-ID: <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com> In-Reply-To: <dcf65725-af6f-4a89-bcac-b1ca46d49cae@www.fastmail.com> References: <CABXB=RRM7YusQL1gGVGD4s9xi9AB4F5AR5-Jqbp4G1WTZWWA%2BQ@mail.gmail.com> <20200826144013.GV2551@kib.kiev.ua> <dcf65725-af6f-4a89-bcac-b1ca46d49cae@www.fastmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Aug 26, 2020 at 12:41 PM Brandon Bergren <bdragon@freebsd.org> wrote:
> The pidfile_* stuff is meant for the daemon side.
As written, agreed, it clearly is. But alas "meant for" and "used
for" appear to differ. It looks like there's a client-side use in ZFS
as well (cddl/compat/opensolaris/misc/fsshare.c).
> Why not just regular open(2)?
That's also a solution I'd be fine with submitting. Certainly less
work. But there are a couple of reasons not to do that, mostly to do
with code reuse.
First, the pidfile library does some stuff around path handling that
is useful not to try to reinvent.
Second, while races do exist, use of pidfiles in this way probably
isn't going away, so having the code that tries to minimize those
races in one place makes a lot of sense, rather than having
potentially multiple idiosyncratic approaches/cases on an ad-hoc
basis. ("Don't let the perfect be the enemy of the good" and all
that.)
The CDDL thing is actually a good example of this, because it looks
lifted straight from mount.c, but when mount.c got updated with the
mountdpid <= 0 check, fsshare.c didn't, and that's almost certainly a
bug because it could lead to inadvertent kill( -1, SIGHUP ) with
superuser permissions. Whoops! ZFS just randomly HUP'd every process
on the system.
Having mount HUP mountd (iff it's running) seems odd to me anyway, but
today's filesystems are so weird and complex and diverse that I'm sure
it's essential for somebody.
Thanks!
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A>