Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 26 Aug 2020 21:15:20 +0000
From:      Rick Macklem <rmacklem@uoguelph.ca>
To:        J David <j.david.lists@gmail.com>, Brandon Bergren <bdragon@FreeBSD.org>
Cc:        FreeBSD Hackers <freebsd-hackers@freebsd.org>
Subject:   Re: pidfile_open() usage in "mount"
Message-ID:  <QB1PR01MB3364C5F4640D161DEE96E579DD540@QB1PR01MB3364.CANPRD01.PROD.OUTLOOK.COM>
In-Reply-To: <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com>
References:  <CABXB=RRM7YusQL1gGVGD4s9xi9AB4F5AR5-Jqbp4G1WTZWWA%2BQ@mail.gmail.com> <20200826144013.GV2551@kib.kiev.ua> <dcf65725-af6f-4a89-bcac-b1ca46d49cae@www.fastmail.com>, <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Brandon Bergren wrote:=0A=
>> The pidfile_* stuff is meant for the daemon side.=0A=
>=0A=
>As written, agreed, it clearly is.  But alas "meant for" and "used=0A=
>for" appear to differ.  It looks like there's a client-side use in ZFS=0A=
>as well (cddl/compat/opensolaris/misc/fsshare.c).=0A=
>=0A=
>> Why not just regular open(2)?=0A=
>=0A=
>That's also a solution I'd be fine with submitting.  Certainly less=0A=
>work.  But there are a couple of reasons not to do that, mostly to do=0A=
>with code reuse.=0A=
>=0A=
>First, the pidfile library does some stuff around path handling that=0A=
>is useful not to try to reinvent.=0A=
>=0A=
>Second, while races do exist, use of pidfiles in this way probably=0A=
>isn't going away, so having the code that tries to minimize those=0A=
>races in one place makes a lot of sense, rather than having=0A=
>potentially multiple idiosyncratic approaches/cases on an ad-hoc=0A=
>basis.  ("Don't let the perfect be the enemy of the good" and all=0A=
>that.)=0A=
>=0A=
>The CDDL thing is actually a good example of this, because it looks=0A=
>lifted straight from mount.c, but when mount.c got updated with the=0A=
>mountdpid <=3D 0 check, fsshare.c didn't, and that's almost certainly a=0A=
>bug because it could lead to inadvertent kill( -1, SIGHUP ) with=0A=
>superuser permissions.  Whoops!  ZFS just randomly HUP'd every process=0A=
>on the system.=0A=
>=0A=
>Having mount HUP mountd (iff it's running) seems odd to me anyway, but=0A=
>today's filesystems are so weird and complex and diverse that I'm sure=0A=
>it's essential for somebody.=0A=
Yes, it is weird and I don't think I was the guilty party.=0A=
The reason is that, if the local file system being mounted happens to be in=
=0A=
/etc/exports, then /etc/exports has to be re-loaded for the update.=0A=
=0A=
Ideally, all local file systems would be mounted and then one HUP would=0A=
be sent to mountd, but that isn't the way it has worked for a long time=0A=
and changing it would be a POLA violation, imho.=0A=
=0A=
Imagine how many times this happens for a server with 20,000 file=0A=
systems that happen to be exported (such a server does exist, apparently).=
=0A=
=0A=
I ended up coming up with an incremental exports update to improve this=0A=
and Peter (the guy that has the 20,000+ file system servers) has a patch th=
at=0A=
uses a database instead of the flat exports file. (Hopefully a variant of h=
is=0A=
patch can make it into ZFS someday. Now that the big switch of ZFS to ZoL=
=0A=
has happened, maybe someday soon?)=0A=
=0A=
rick=0A=
=0A=
Thanks!=0A=
_______________________________________________=0A=
freebsd-hackers@freebsd.org mailing list=0A=
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers=0A=
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"=
=0A=
=0A=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QB1PR01MB3364C5F4640D161DEE96E579DD540>