From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Subject: Re: [PATCH XEN v6 29/32] tools/libs/call: Use O_CLOEXEC when opening /dev/xen/privcmd on Linux Date: Sat, 12 Dec 2015 11:55:05 +0100 Message-ID: <566BFD09.7070703@citrix.com> References: <1449141675.4424.125.camel@citrix.com> <1449141749-14940-1-git-send-email-ian.campbell@citrix.com> <1449141749-14940-30-git-send-email-ian.campbell@citrix.com> <22120.21503.563186.915788@mariner.uk.xensource.com> <1449854573.30975.85.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1449854573.30975.85.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Ian Jackson Cc: wei.liu2@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org El 11/12/15 a les 18.22, Ian Campbell ha escrit: > On Wed, 2015-12-09 at 16:17 +0000, Ian Jackson wrote: >> Ian Campbell writes ("[PATCH XEN v6 29/32] tools/libs/call: Use O_CLOEXEC >> when opening /dev/xen/privcmd on Linux"): >>> We stick with using FD_CLOEXEC on the legacy /proc/xen/privcmd >>> fallback device since it was present in older kernel where O_CLOEXEC >>> may not be assured. >> >> This is a lot of effort and may not be needed. I don't object, but >> some of the statements are (I think) rather too fierce: >> >>> + /* >>> + * This file descriptor is opaque to the caller, thus we must take >>> + * responsibility to ensure it doesn't propagate (ie leak) outside >>> + * the process, by using CLOEXEC. >>> + */ >> >> For example, I don't think this is a `must' at all, although not >> propagating irrelevant fds is (nowadays) seen as polite. > > Right, this bit was actually (mostly) code^Wcomment motion. > > I'll happily update it to say polite rather than required. > > I did a bit more investigation of O_CLOEXEC and found that Linux introduced > it in 2.6.23 (October 2007) which seems to be old enough that we should > just use it anywhere we feel it necessary. > > Jan did mention (on IRC) that while he doesn't use any kernels so old, he > still occasionally builds on userspace which is old enough to lack the > definition, hence I would do #ifndef+#define. > > Roger, I see that open(2) on FreeBSD mentions O_CLOEXEC too. Do you know > when that arrived and whether it is something we can assume these days? > I found a posting of the patch in https://lists.freebsd.org/pipermail/freeb > sd-fs/2011-March/011021.html, but I don't know when it landed or when it > became something code could assume. privcmd is only available on FreeBSD HEAD anyway, and AFAICT (by looking at the man pages) all stable FreeBSD versions support O_CLOEXEC (10.2, 9.3 and 8.4). I think we can safely assume O_CLOEXEC is available in all FreeBSD version we build on. Roger.