All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pádraig Brady" <P@draigBrady.com>
To: Yann Droneaud <ydroneaud@opteya.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Eric Paris <eparis@redhat.com>,
	Richard Guy Briggs <rgb@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	stable@vger.kernel.org, linux-api@vger.kernel.org,
	Jan Kara <jack@suse.cz>, Lino Sanfilippo <LinoSanfilippo@gmx.de>,
	Valdis Kletnieks <Valdis.Kletnieks@vt.edu>,
	Michael Kerrisk-manpages <mtk.manpages@gmail.com>
Subject: Re: [PATCH] fanotify: add a flag to allow setting O_CLOEXEC on event fd
Date: Thu, 02 Oct 2014 10:13:10 +0100	[thread overview]
Message-ID: <542D1726.3040801@draigBrady.com> (raw)
In-Reply-To: <1412236349-30035-1-git-send-email-ydroneaud@opteya.com>

On 10/02/2014 08:52 AM, Yann Droneaud wrote:
> In order to not potentially break applications which were
> requesting O_CLOEXEC on event file descriptors but which
> actually need it to be not effective as the kernel currently
> ignore the flag, so the file descriptor is inherited accross
> exec regardless of O_CLOEXEC (please forgive me for the
> wording), this patch introduces FAN_FD_CLOEXEC flag to
> fanotify_init() so that application can request O_CLOEXEC
> to be effective.
> Newer application would use FAN_FD_CLOEXEC flag along
> O_CLOEXEC to enable close on exec on newly created
> file descriptor:
> 
>   fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK|FAN_FD_CLOEXEC,
>                      O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);

Ugh really?
IMHO there should be widespread or at least known breakage with
O_CLOEXEC before adding messiness like this.
It seems surprising to me that apps that would depend on
O_CLOEXEC being ineffective.

please reconsider this one.

thanks,
Pádraig.


WARNING: multiple messages have this Message-ID (diff)
From: "Pádraig Brady" <P@draigBrady.com>
To: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>,
	Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
	Lino Sanfilippo <LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org>,
	Valdis Kletnieks <Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org>,
	Michael Kerrisk-manpages
	<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH] fanotify: add a flag to allow setting O_CLOEXEC on event fd
Date: Thu, 02 Oct 2014 10:13:10 +0100	[thread overview]
Message-ID: <542D1726.3040801@draigBrady.com> (raw)
In-Reply-To: <1412236349-30035-1-git-send-email-ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>

On 10/02/2014 08:52 AM, Yann Droneaud wrote:
> In order to not potentially break applications which were
> requesting O_CLOEXEC on event file descriptors but which
> actually need it to be not effective as the kernel currently
> ignore the flag, so the file descriptor is inherited accross
> exec regardless of O_CLOEXEC (please forgive me for the
> wording), this patch introduces FAN_FD_CLOEXEC flag to
> fanotify_init() so that application can request O_CLOEXEC
> to be effective.
> Newer application would use FAN_FD_CLOEXEC flag along
> O_CLOEXEC to enable close on exec on newly created
> file descriptor:
> 
>   fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK|FAN_FD_CLOEXEC,
>                      O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);

Ugh really?
IMHO there should be widespread or at least known breakage with
O_CLOEXEC before adding messiness like this.
It seems surprising to me that apps that would depend on
O_CLOEXEC being ineffective.

please reconsider this one.

thanks,
Pádraig.

WARNING: multiple messages have this Message-ID (diff)
From: "Pádraig Brady" <P@draigBrady.com>
To: Yann Droneaud <ydroneaud@opteya.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Eric Paris <eparis@redhat.com>,
	Richard Guy Briggs <rgb@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	stable@vger.kernel.org, linux-api@vger.kernel.org,
	Jan Kara <jack@suse.cz>, Lino Sanfilippo <LinoSanfilippo@gmx.de>,
	Valdis Kletnieks <Valdis.Kletnieks@vt.edu>,
	Michael Kerrisk-manpages <mtk.manpages@gmail.com>
Subject: Re: [PATCH] fanotify: add a flag to allow setting O_CLOEXEC on event fd
Date: Thu, 02 Oct 2014 10:13:10 +0100	[thread overview]
Message-ID: <542D1726.3040801@draigBrady.com> (raw)
In-Reply-To: <1412236349-30035-1-git-send-email-ydroneaud@opteya.com>

On 10/02/2014 08:52 AM, Yann Droneaud wrote:
> In order to not potentially break applications which were
> requesting O_CLOEXEC on event file descriptors but which
> actually need it to be not effective as the kernel currently
> ignore the flag, so the file descriptor is inherited accross
> exec regardless of O_CLOEXEC (please forgive me for the
> wording), this patch introduces FAN_FD_CLOEXEC flag to
> fanotify_init() so that application can request O_CLOEXEC
> to be effective.
> Newer application would use FAN_FD_CLOEXEC flag along
> O_CLOEXEC to enable close on exec on newly created
> file descriptor:
> 
>   fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK|FAN_FD_CLOEXEC,
>                      O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);

Ugh really?
IMHO there should be widespread or at least known breakage with
O_CLOEXEC before adding messiness like this.
It seems surprising to me that apps that would depend on
O_CLOEXEC being ineffective.

please reconsider this one.

thanks,
P�draig.


  reply	other threads:[~2014-10-02  9:14 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 13:11 [PATCHv8 0/6] Getting rid of get_unused_fd() / enable close-on-exec Yann Droneaud
2014-09-24 13:11 ` Yann Droneaud
2014-09-24 13:11 ` Yann Droneaud
2014-09-24 13:11 ` Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 1/6] fanotify: enable close-on-exec on events' fd when requested in fanotify_init() Yann Droneaud
2014-09-25 20:57   ` Heinrich Schuchardt
2014-09-25 20:57     ` Heinrich Schuchardt
2014-09-26  8:58     ` Yann Droneaud
2014-09-27  7:26       ` Heinrich Schuchardt
2014-09-29  8:49         ` [PATCHv8.1] " Yann Droneaud
2014-09-29 11:50           ` Jan Kara
2014-10-01 22:36           ` Andrew Morton
2014-10-01 22:36             ` Andrew Morton
2014-10-02  6:20             ` Yann Droneaud
2014-10-02  6:50               ` Mihai Donțu
2014-10-02  7:52               ` [PATCH] fanotify: add a flag to allow setting O_CLOEXEC on event fd Yann Droneaud
2014-10-02  9:13                 ` Pádraig Brady [this message]
2014-10-02  9:13                   ` Pádraig Brady
2014-10-02  9:13                   ` Pádraig Brady
2014-10-02 11:55                   ` Michael Kerrisk (man-pages)
2014-10-02 14:44                   ` Yann Droneaud
2014-10-02 14:44                     ` Yann Droneaud
2014-10-02 10:44             ` [PATCHv8.1] fanotify: enable close-on-exec on events' fd when requested in fanotify_init() Jan Kara
2014-10-02 10:44               ` Jan Kara
2014-10-02 19:46               ` Andrew Morton
2014-10-02 19:46                 ` Andrew Morton
2014-10-03  8:39                 ` [PATCHv8.2] " Yann Droneaud
2014-10-03  8:39                   ` Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 2/6] ia64: trivial: replace get_unused_fd() by get_unused_fd_flags(0) Yann Droneaud
2014-09-24 13:11   ` Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 3/6] ppc/cell: " Yann Droneaud
2014-09-24 13:11   ` Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 4/6] binfmt_misc: " Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 5/6] file: " Yann Droneaud
2014-09-24 13:11 ` [PATCHv8 6/6] file: remove get_unused_fd() macro Yann Droneaud

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 \
    --in-reply-to=542D1726.3040801@draigBrady.com \
    --to=p@draigbrady.com \
    --cc=LinoSanfilippo@gmx.de \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=eparis@redhat.com \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=rgb@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xypron.glpk@gmx.de \
    --cc=ydroneaud@opteya.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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.