Re: install(1) could fail due to race

2019-02-08 Thread Ingo Schwarze
Ted Unangst wrote on Sun, Jan 27, 2019 at 10:37:52AM -0500: > Ingo Schwarze wrote: >> If people here agree with the general direction of making -S the >> default and removing the fragile non-S mode (see the patch below), >> i'll run a full make build and make release and then ask for OKs. > Just

Re: install(1) could fail due to race

2019-02-06 Thread Ingo Schwarze
Hi Ted, Ted Unangst wrote on Sun, Jan 27, 2019 at 10:37:52AM -0500: > Ingo Schwarze wrote: >> If people here agree with the general direction of making -S the >> default and removing the fragile non-S mode (see the patch below), >> i'll run a full make build and make release and then ask for

Re: install(1) could fail due to race

2019-02-06 Thread Lauri Tirkkonen
On Sun, Jan 27 2019 10:37:52 -0500, Ted Unangst wrote: > Ingo Schwarze wrote: > > If people here agree with the general direction of making -S the > > default and removing the fragile non-S mode (see the patch below), > > i'll run a full make build and make release and then ask for OKs. > > Just

Re: install(1) could fail due to race

2019-01-27 Thread Ted Unangst
Ingo Schwarze wrote: > If people here agree with the general direction of making -S the > default and removing the fragile non-S mode (see the patch below), > i'll run a full make build and make release and then ask for OKs. Just checking we didn't forget about this. Seems the right thing to do.

Re: install(1) could fail due to race

2019-01-16 Thread Lauri Tirkkonen
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. > > > >

Re: install(1) could fail due to race

2019-01-16 Thread Ingo Schwarze
Hi, 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

Re: install(1) could fail due to race

2019-01-10 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 14:27:29 -0700, Todd C. Miller wrote: > The main difference with -S is that you end up with two copies of > the target file on disk for a short period of time. That was a > bigger deal back when -S was added. Today, it probably doesn't > matter. so, how about this patch

Re: install(1) could fail due to race

2019-01-07 Thread Theo de Raadt
Ted Unangst wrote: > Todd C. Miller wrote: > > On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > > > > > Oooh, that short period can be the time between disk flushes (30s?) > > > with softupdates. I think that's one of the scenarios where you > > > can run out of disk space if

Re: install(1) could fail due to race

2019-01-07 Thread Ted Unangst
Todd C. Miller wrote: > On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > > > Oooh, that short period can be the time between disk flushes (30s?) > > with softupdates. I think that's one of the scenarios where you > > can run out of disk space if you have softupdates enabled. >

Re: install(1) could fail due to race

2019-01-07 Thread Todd C . Miller
On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > Oooh, that short period can be the time between disk flushes (30s?) > with softupdates. I think that's one of the scenarios where you > can run out of disk space if you have softupdates enabled. Yes, I think I've had that happen

Re: install(1) could fail due to race

2019-01-07 Thread Christian Weisgerber
On 2019-01-07, "Todd C. Miller" wrote: > The main difference with -S is that you end up with two copies of > the target file on disk for a short period of time. Oooh, that short period can be the time between disk flushes (30s?) with softupdates. I think that's one of the scenarios where you

Re: install(1) could fail due to race

2019-01-07 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 16:32:31 -0500, Ted Unangst wrote: > > > This doubles the number of synchronous > > > file operations. > > > > Does it? Without safecopy, the operations performed are: > > > > unlink(targetfile); > > open(targetfile, O_CREAT|O_EXCL); > > write(); > > fchmod();

Re: install(1) could fail due to race

2019-01-07 Thread Ted Unangst
Lauri Tirkkonen wrote: > On Mon, Jan 07 2019 15:48:25 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > 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. > > > > > The below diff

Re: install(1) could fail due to race

2019-01-07 Thread Todd C . Miller
The main difference with -S is that you end up with two copies of the target file on disk for a short period of time. That was a bigger deal back when -S was added. Today, it probably doesn't matter. - todd

Re: install(1) could fail due to race

2019-01-07 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 15:48:25 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > 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. > > > The below diff essentially removes the -S option and

Re: install(1) could fail due to race

2019-01-07 Thread Ted Unangst
Lauri Tirkkonen wrote: > 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. > The below diff essentially removes the -S option and makes install > always use temp files (ie. -S is always on),

Re: install(1) could fail due to race

2019-01-07 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 20:13:09 +0200, Lauri Tirkkonen wrote: > @@ -128,9 +127,6 @@ main(int argc, char *argv[]) > case 'p': > docompare = dopreserve = 1; > break; > - case 'S': > - safecopy = 1; > -

install(1) could fail due to race

2019-01-07 Thread Lauri Tirkkonen
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, but I actually ran into this when parallel building gcc 6.4.0 (on another OS, but we use OpenBSD install(1)): they