linux-kernel - Re: O_CLOFORK use case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 03 Jun 2019 18:56:49 +0300
From:   Mihai Donțu <mihai.dontu@...il.com>
To:     Joshua Hudson <joshudson@...il.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: O_CLOFORK use case

On Mon, 2019-06-03 at 08:24 -0700, Joshua Hudson wrote:
> I ran headlong into the use case for O_CLOFORK and got into a locking
> debate over it.
> 
> The actual use case involves squashing a thread race between two
> threads. If a file is opened for write in one thread with O_CLOEXEC
> while another thread calls fork(), a race condition can happen where
> the thread that closes the handle misses the out of disk error because
> the child process closed the handle last inside execve().
> 
> The decades old idiom for replacing config files isn't safe in
> multi-threaded code. Yipe.
> 
>     int h = open(".configfile~", O_WRONY | O_EXCL | O_CLOEXEC, 0666);
>     if (h < 0) { perror(".configfile"); return 1; }
>     ssize_t delta = 0;
>     while ((delta = write(h, newconfigdata, newconfiglen)) > 0) {
>         newconfigdata += delta;
>         newconfiglen -= delta;
>     }
>     if (delta < 0) { perror(".configfile"); return 1; }
>     if (close(h)) { perror(".configfile"); return 1; }
>     rename(".configfile~", ".configfile");
> 
> To fix it, we have to put locks around close() and fork()/vfork(). Ugh.

fsync() / fdatasync() before close() should fix it, non?

I was under the impression that any of those two became mandatory when
ext4 came along.

-- 
Mihai Donțu