> On Jun 21, 2019, at 12:32, Xin LI <delp...@gmail.com> wrote:
>
>
>> On Thu, Jun 20, 2019 at 7:38 PM Conrad Meyer <c...@freebsd.org> wrote:
>> Author: cem
>> Date: Fri Jun 21 02:37:54 2019
>> New Revision: 349256
>> URL: https://svnweb.freebsd.org/changeset/base/349256
>>
>> Log:
>> rc.d/motd: Update motd more robustly
>>
>> Use appropriate fsyncs to persist the rewritten /etc/motd file, when a
>> rewrite is performed.
>
> Why is /etc/motd so important to deserve this kind of construct? The worst
> that could happen to /etc/motd with previous code is that you might end up
> with an empty or non-existent /etc/motd, and the change have introduced more
> problems than it intended to solve.
>
>>
>> Reported by: Jonathan Walton <jonathan AT isilon.com>
>> Reviewed by: allanjude, vangyzen
>> Sponsored by: Dell EMC Isilon
>> Differential Revision: https://reviews.freebsd.org/D20701
>>
>> Modified:
>> head/libexec/rc/rc.d/motd
>>
>> Modified: head/libexec/rc/rc.d/motd
>> ==============================================================================
>> --- head/libexec/rc/rc.d/motd Fri Jun 21 00:52:30 2019 (r349255)
>> +++ head/libexec/rc/rc.d/motd Fri Jun 21 02:37:54 2019 (r349256)
>> @@ -37,11 +37,15 @@ motd_start()
>> uname -v | sed -e 's,^\([^#]*\) #\(.*
>> [1-2][0-9][0-9][0-9]\).*/\([^\]*\) $,\1 (\3) #\2,' > ${T}
>> awk '{if (NR == 1) {if ($1 == "FreeBSD") {next} else {print "\n"$0}}
>> else {print}}' < /etc/motd >> ${T}
>>
>> - cmp -s $T /etc/motd || {
>> - cp $T /etc/motd
>> + if ! cmp -s $T /etc/motd; then
>> + mv -f $T /etc/.motd.tmp
>
> This have partially defeated the benefit of doing mktemp in the code above.
> The old code tries to avoid excessive writes to /, which is preserved, but it
> is not safe to assume /etc/.motd.tmp is a non-existent plain file: mv would
> happily proceed when .motd.tmp is a preexisting directory, for instance (and
> subsequent mv -f would fail). A malicious system administrator can now plant
> a timing bomb which will fill / eventually.
>
> You can do a TMPDIR=/etc/ mktemp .motd.tmp.XXXXXXXX and use that to mitigate
> the attack above, but then if the system crash in the middle you would still
> end up with a dangling .motd.tmp.XXXXXXXX file in /etc.
>
> Also note that $T is on /tmp which is likely to be a different filesystem,
> internally this is mostly equivalent to cp + rm. You can simplify the code
> by reverting to previous cp and keep the rm -f below as an unconditional
> remove like before.
>
>> + fsync /etc/.motd.tmp
>> + mv -f /etc/.motd.tmp /etc/motd
>> chmod ${PERMS} /etc/motd
>> - }
>> - rm -f $T
>> + fsync /etc
>
> There is absolutely no reason to fsync /etc here. mv'ing on the same file
> system is performed with rename(2) which is required to be atomic by POSIX.
> /etc/motd will be pointing to either the old one or the new one in the event
> of a crash, and both are acceptable.
>
> So, in my opinion the new code would have made the situation worse than the
> old code.
>
> But ultimately, I think the real design question here that needs to be solved
> would probably be "Why are piling up multiple layers of workarounds around
> motd? Does it even need to be located in /etc?" The contents is meant to be
> updated every time when there is a kernel change, and to that extent it seems
> to be more appropriate for /var/run and generated at boot from a template
> located somewhere in /etc. The benefit of this approach is that you would
> have one less file to merge for each etcupdate/mergemaster (or at least only
> need to do it when some customization is made), and there is no need to worry
> about write durability.
I don’t know the exact reasoning today, but once upon a time, there used to
be a number of things hooked to /etc/motd generation that could influence
information provided to downstream callers/systems.
Thank you,
-Enji
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
