On Wed, Jan 16 2019 11:00:04 +0100, Ingo Schwarze wrote: > Lauri Tirkkonen wrote on Mon, Jan 07, 2019 at 08:13:09PM +0200: > > > Hi, it seems install(1) has a race condition: in create_newfile, it > > first unlinks the target file and then tries to open it with > > O_CREAT|O_EXCL. > > > > Normally this would not be a problem, > > A race condition is almost always a problem and can almost always > cause incorrect behaviour. In this case, *anything* might create > the file in the time window, causing spurious failures, in unusual > scenarios maybe even facilitating DOS attacks. The fact that some > build systems shoot themselves in the foot the way you describe > merely exacerbates the bug.
Yes, you're right. > > The below diff essentially removes the -S option > > We clearly cannot delete -S from the user interface. That would break > all kinds of build systems for no benefit. It is even used in base, > for example in bsd.lib.mk. I don't doubt it's used in ports, too, > and having to fix it there would be even more annoying than in base. The second diff I sent removes the documentation, but keeps accepting the option as a no-op just for that reason. I don't see much point in documenting it since you cannot turn it off, but it's your call, of course. > > and makes install always use temp files (ie. -S is always on), > > eliminating the race since rename(2) cannot fail like this. > > I think that is the right thing to do. Even if it happens to > increase resource consumption with softupdates, that is of little > relevance because we advise against using softupdates in the first > place, for reasons of reliability. Also, bugs in one subsystem > must not prevent bugfixing in another. Right, that's why I said I think softupdates is orthogonal. Besides the default mk files already use -S, so there should be no behavior change for building base. -- Lauri Tirkkonen | lotheac @ IRCnet
