From: "Karstens, Nate" <Nate.Karstens@garmin.com> To: James Bottomley <James.Bottomley@HansenPartnership.com>, Matthew Wilcox <email@example.com> Cc: Alexander Viro <firstname.lastname@example.org>, Jeff Layton <email@example.com>, "J. Bruce Fields" <firstname.lastname@example.org>, Arnd Bergmann <email@example.com>, Richard Henderson <firstname.lastname@example.org>, Ivan Kokshaysky <email@example.com>, Matt Turner <firstname.lastname@example.org>, Helge Deller <email@example.com>, "David S. Miller" <firstname.lastname@example.org>, Jakub Kicinski <email@example.com>, Eric Dumazet <firstname.lastname@example.org>, David Laight <David.Laight@aculab.com>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, Changli Gao <email@example.com>, "firstname.lastname@example.org" <email@example.com> Subject: RE: [PATCH v2] Implement close-on-fork Date: Fri, 15 May 2020 18:28:20 +0000 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <1589559950.3653.11.camel@HansenPartnership.com> James, Sorry, perhaps I was indirect, but I thought I had responded to that in https://email@example.com/. I really hope I do not come off as complaining about this issue. We identified what seemed to be something that was overlooked with the various APIs around creating child processes. Rather than fixing it ourselves and moving on we chose to invest more time and effort into it by engaging the community (first POSIX, and now this one) in a discussion. I humbly and sincerely ask if you would help me understand, if we could turn back the clock, how our application could have been written to avoid this issue: *A parent process forks a child. Another thread in the parent process closes and attempts to reopen a socket, file, or other resource it needs exclusive access to. This fails because the -operating system- still has a reference to that resource that it is keeping on behalf of the child. The child eventually calls exec and the resource is closed because the close-on-exec flag is set.* Our first attempt, which was to use the pthread_atfork() handlers, failed because system() is not required to call the handlers. Most of the feedback we're getting on this seems to say "don't use system(), it is unsafe for threaded applications". Is that documented anywhere? The man page says it is "MT-Safe". Aside from that, even if we remove all uses of system() from our application (which we already have), then our application, like many other applications, needs to use third-party shared libraries. There is nothing that prevents those libraries from using system(). We can audit those libraries and go back with the vendor with a request to replace system() with a standard fork/exec, but they will also want documentation supporting that. We can also take steps to change or remove system() from our standard library. It fixes our issue, but still leaves the community with an API that is broken/flawed/poorly-documented (depending on how one looks at it). If the feedback from the community is truly and finally that system() should not be used in these applications, then is there support for updating the man page to better communicate that? Thanks for your help with this. Nate -----Original Message----- From: James Bottomley <James.Bottomley@HansenPartnership.com> Sent: Friday, May 15, 2020 11:26 To: Karstens, Nate <Nate.Karstens@garmin.com>; Matthew Wilcox <firstname.lastname@example.org> Cc: Alexander Viro <email@example.com>; Jeff Layton <firstname.lastname@example.org>; J. Bruce Fields <email@example.com>; Arnd Bergmann <firstname.lastname@example.org>; Richard Henderson <email@example.com>; Ivan Kokshaysky <firstname.lastname@example.org>; Matt Turner <email@example.com>; Helge Deller <firstname.lastname@example.org>; David S. Miller <email@example.com>; Jakub Kicinski <firstname.lastname@example.org>; Eric Dumazet <email@example.com>; David Laight <David.Laight@aculab.com>; firstname.lastname@example.org; email@example.com; firstname.lastname@example.org; email@example.com; firstname.lastname@example.org; email@example.com; firstname.lastname@example.org; Changli Gao <email@example.com>; firstname.lastname@example.org Subject: Re: [PATCH v2] Implement close-on-fork CAUTION - EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe. On Fri, 2020-05-15 at 16:07 +0000, Karstens, Nate wrote: > Matthew, > > What alternative would you suggest? > > From an earlier email: > > > ...nothing else addresses the underlying issue: there is no way to > > prevent a fork() from duplicating the resource. The close-on-exec > > flag partially-addresses this by allowing the parent process to mark > > a file descriptor as exclusive to itself, but there is still a > > period of time the failure can occur because the auto-close only > > occurs during the exec(). Perhaps this would not be an issue with a > > different process/threading model, but that is another discussion > > entirely. > > Do you disagree there is an issue? Oh good grief that's a leading question: When I write bad code and it crashes, most people would agree there is an issue; very few would agree the kernel should be changed to fix it. Several of us have already said the problem seems to be with the way your application is written. You didn't even answer emails like this speculating about the cause being the way your application counts resources: https://lore.kernel.org/linux-fsdevel/1587569663.3485.18.camel@HansenPartnership.com/ The bottom line is that we think you could rewrite this one application not to have the problem you're complaining about rather than introduce a new kernel API to "fix" it. James ________________________________ CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
next prev parent reply index Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-15 15:23 Nate Karstens 2020-05-15 15:23 ` [PATCH v2 1/4] fs: " Nate Karstens 2020-05-15 15:23 ` [PATCH v2 2/4] fs: Add O_CLOFORK flag for open(2) and dup3(2) Nate Karstens 2020-05-15 15:23 ` [PATCH v2 3/4] fs: Add F_DUPFD_CLOFORK to fcntl(2) Nate Karstens 2020-05-15 15:23 ` [PATCH v2 4/4] net: Add SOCK_CLOFORK Nate Karstens 2020-05-15 15:30 ` [PATCH v2] Implement close-on-fork Eric Dumazet 2020-05-15 15:59 ` David Laight 2020-05-15 15:57 ` Matthew Wilcox 2020-05-15 16:07 ` Karstens, Nate 2020-05-15 16:25 ` James Bottomley 2020-05-15 18:28 ` Karstens, Nate [this message] 2020-05-15 18:43 ` Matthew Wilcox 2020-05-25 8:16 ` Pavel Machek 2020-05-15 16:26 ` Matthew Wilcox 2020-05-16 13:29 ` Christian Brauner 2020-05-15 16:03 ` Al Viro 2020-05-15 16:26 ` Karstens, Nate 2020-05-15 16:53 ` David Howells
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --cc=David.Laight@aculab.com \ --cc=James.Bottomley@HansenPartnership.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-parisc archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-parisc/0 linux-parisc/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-parisc linux-parisc/ https://lore.kernel.org/linux-parisc \ firstname.lastname@example.org public-inbox-index linux-parisc Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-parisc AGPL code for this site: git clone https://public-inbox.org/public-inbox.git