Date: Fri, 20 Jan 2017 18:46:31 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: Antoine Brodin <antoine@freebsd.org>, Xin LI <delphij@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r312404 - head/usr.bin/sed Message-ID: <20170120182151.E2080@besplex.bde.org> In-Reply-To: <20170120174258.X1938@besplex.bde.org> References: <201701190801.v0J81ZG9008267@repo.freebsd.org> <CAALwa8neni57SPZAo4jW8wKu-Up0-M=weR31WtndzXdW2_jKmQ@mail.gmail.com> <20170120174258.X1938@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 20 Jan 2017, Bruce Evans wrote: > On Fri, 20 Jan 2017, Antoine Brodin wrote: >> sed -i no longer works on symlinks which breaks lots of ports. >> Please revert and request an exp-run. > > sed() doesn't seem to actually understand symlinks (it has no flag like > -h to prevent following them when opening files), so its use of lstat() > to classify them is wrong. This was harmless for symlinks but not for Oops. It only does the lstat() classification for the -I and -i cases, where restricting to regular files is not completely wrong, but I don't see any reason to do it. Just require the target of the symlink to be regular (and still be wrong when it is an irregular regular file in /proc). The check also has races. These may be hard to avoid if symlinks must not be followed, but since sed must follow symlinks the correct method is to open the file (following symlinks) and fstat() the fd. sed has special support for stdin and stdout and special checks to disallow -I and -i on stdin and stdout. The fstat() check on them wouldn't work right. Editing stdin is only impossible since you can't reopen it for both input and output. sed has related problems and races in its supported case whee reopening is possible since the file name is know. A non-racy version might open the file in r+ mode, then fstat() it, but sed actually uses separate streams and even a tempoary file, so it is not very in-place and has the usual races with temporary files. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170120182151.E2080>