[libmnl] Creating netlink socket with SOCK_CLOEXEC flag

[libmnl] Creating netlink socket with SOCK_CLOEXEC flag

[Guillaume Nault]

Hi,

I'm looking at how libmnl could be used with CLOEXEC netlink sockets.
Of course, one can use the
   nl = mnl_socket_open();
   fd = mnl_socket_get_fd(nl);
   fcntl(fd, F_SETFD, O_CLOEXEC);
sequence, but this is racy in multi-threaded programs, where another
thread could fork()/execve() between the mnl_socket_open() and the
fcntl() calls. Applying the CLOEXEC flag at socket creation closes this
issue.

There are three different approaches I can think of:
  1- Make mnl_socket_open() unconditionally add the SOCK_CLOEXEC flag in
     its socket() call.
  2- Define mnl_socket_open2(), similar to mnl_socket_open() but with an
     additional flags parameter that would be passed to socket().
  3- Tell user to create its netlink socket with the required flags and
     use it with mnl_socket_fdopen().

Solution #1 would provide safe default for all users, but that'd be an
ABI change. Also decision would need to be made wrt. platforms not
handling SOCK_CLOEXEC.

Solution #2 is more generic and allows all SOCK_* flags defined by the
plateform. But it's a bit more inelegant and exports yet another function
to allocate an mnl socket.

Solution #3 doesn't require any libmnl code update, but isn't (yet?) part
of any libmnl release. It also makes mnl socket creation not that well
integrated in libmnl.

Is any of these solutions acceptable? Any other idea on how to guarantee
race free application of the CLOEXEC flag to libmnl sockets?


Regards,

Guillaume

Re: [libmnl] Creating netlink socket with SOCK_CLOEXEC flag

[Pablo Neira Ayuso]

On Thu, Oct 01, 2015 at 06:16:04PM +0200, Guillaume Nault wrote:
> Hi,
> 
> I'm looking at how libmnl could be used with CLOEXEC netlink sockets.
> Of course, one can use the
>    nl = mnl_socket_open();
>    fd = mnl_socket_get_fd(nl);
>    fcntl(fd, F_SETFD, O_CLOEXEC);
> sequence, but this is racy in multi-threaded programs, where another
> thread could fork()/execve() between the mnl_socket_open() and the
> fcntl() calls. Applying the CLOEXEC flag at socket creation closes this
> issue.
> 
> There are three different approaches I can think of:
>   1- Make mnl_socket_open() unconditionally add the SOCK_CLOEXEC flag in
>      its socket() call.
>   2- Define mnl_socket_open2(), similar to mnl_socket_open() but with an
>      additional flags parameter that would be passed to socket().
>   3- Tell user to create its netlink socket with the required flags and
>      use it with mnl_socket_fdopen().
> 
> Solution #1 would provide safe default for all users, but that'd be an
> ABI change. Also decision would need to be made wrt. platforms not
> handling SOCK_CLOEXEC.
> 
> Solution #2 is more generic and allows all SOCK_* flags defined by the
> plateform. But it's a bit more inelegant and exports yet another function
> to allocate an mnl socket.

I'd suggest you add mnl_socket_open2().

Re: [libmnl] Creating netlink socket with SOCK_CLOEXEC flag

[Guillaume Nault]

On Thu, Oct 01, 2015 at 06:50:05PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Oct 01, 2015 at 06:16:04PM +0200, Guillaume Nault wrote:
> > Hi,
> > 
> > I'm looking at how libmnl could be used with CLOEXEC netlink sockets.
> > Of course, one can use the
> >    nl = mnl_socket_open();
> >    fd = mnl_socket_get_fd(nl);
> >    fcntl(fd, F_SETFD, O_CLOEXEC);
> > sequence, but this is racy in multi-threaded programs, where another
> > thread could fork()/execve() between the mnl_socket_open() and the
> > fcntl() calls. Applying the CLOEXEC flag at socket creation closes this
> > issue.
> > 
> > There are three different approaches I can think of:
> >   1- Make mnl_socket_open() unconditionally add the SOCK_CLOEXEC flag in
> >      its socket() call.
> >   2- Define mnl_socket_open2(), similar to mnl_socket_open() but with an
> >      additional flags parameter that would be passed to socket().
> >   3- Tell user to create its netlink socket with the required flags and
> >      use it with mnl_socket_fdopen().
> > 
> > Solution #1 would provide safe default for all users, but that'd be an
> > ABI change. Also decision would need to be made wrt. platforms not
> > handling SOCK_CLOEXEC.
> > 
> > Solution #2 is more generic and allows all SOCK_* flags defined by the
> > plateform. But it's a bit more inelegant and exports yet another function
> > to allocate an mnl socket.
> 
> I'd suggest you add mnl_socket_open2().

Ok, will do.