* [PATCH] ip: set the close-on-exec flag for descriptors
@ 2013-06-04 8:01 Andrey Vagin
2013-06-04 16:11 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Vagin @ 2013-06-04 8:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Andrey Vagin
Otherwise a program executed by "ip netns exec" has two extra
descriptors.
$ ip netns exec test /bin/bash
$ lsof -p $$
...
bash 817 root 0u CHR 136,0 0t0 3 /dev/pts/0
bash 817 root 1u CHR 136,0 0t0 3 /dev/pts/0
bash 817 root 2u CHR 136,0 0t0 3 /dev/pts/0
bash 817 root 3u sock 0,6 0t0 13386 protocol: NETLINK
bash 817 root 4r REG 0,3 0 4026532155 net
bash 817 root 255u CHR 136,0 0t0 3 /dev/pts/0
Cc: Stephen Hemminger <stephen@networkplumber.org>
Reported-by: Dilip Daya <dilip.daya@hp.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
ip/ipnetns.c | 2 +-
lib/libnetlink.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index c9bc20a..fa2b681 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -150,7 +150,7 @@ static int netns_exec(int argc, char **argv)
name = argv[0];
cmd = argv[1];
snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
- netns = open(net_path, O_RDONLY);
+ netns = open(net_path, O_RDONLY | O_CLOEXEC);
if (netns < 0) {
fprintf(stderr, "Cannot open network namespace \"%s\": %s\n",
name, strerror(errno));
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index b17e1aa..9e2a795 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -43,7 +43,7 @@ int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions,
memset(rth, 0, sizeof(*rth));
- rth->fd = socket(AF_NETLINK, SOCK_RAW, protocol);
+ rth->fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, protocol);
if (rth->fd < 0) {
perror("Cannot open netlink socket");
return -1;
--
1.8.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-06-04 8:01 [PATCH] ip: set the close-on-exec flag for descriptors Andrey Vagin
@ 2013-06-04 16:11 ` Stephen Hemminger
2013-07-08 14:17 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2013-06-04 16:11 UTC (permalink / raw)
To: Andrey Vagin; +Cc: netdev
On Tue, 4 Jun 2013 12:01:14 +0400
Andrey Vagin <avagin@openvz.org> wrote:
> Otherwise a program executed by "ip netns exec" has two extra
> descriptors.
>
> $ ip netns exec test /bin/bash
> $ lsof -p $$
> ...
> bash 817 root 0u CHR 136,0 0t0 3 /dev/pts/0
> bash 817 root 1u CHR 136,0 0t0 3 /dev/pts/0
> bash 817 root 2u CHR 136,0 0t0 3 /dev/pts/0
> bash 817 root 3u sock 0,6 0t0 13386 protocol: NETLINK
> bash 817 root 4r REG 0,3 0 4026532155 net
> bash 817 root 255u CHR 136,0 0t0 3 /dev/pts/0
Applied
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-06-04 16:11 ` Stephen Hemminger
@ 2013-07-08 14:17 ` Eric Dumazet
2013-07-08 15:51 ` Andrey Wagin
2013-07-08 19:57 ` Stephen Hemminger
0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2013-07-08 14:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Andrey Vagin, netdev
On Tue, 2013-06-04 at 09:11 -0700, Stephen Hemminger wrote:
> On Tue, 4 Jun 2013 12:01:14 +0400
> Andrey Vagin <avagin@openvz.org> wrote:
>
> > Otherwise a program executed by "ip netns exec" has two extra
> > descriptors.
> >
> > $ ip netns exec test /bin/bash
> > $ lsof -p $$
> > ...
> > bash 817 root 0u CHR 136,0 0t0 3 /dev/pts/0
> > bash 817 root 1u CHR 136,0 0t0 3 /dev/pts/0
> > bash 817 root 2u CHR 136,0 0t0 3 /dev/pts/0
> > bash 817 root 3u sock 0,6 0t0 13386 protocol: NETLINK
> > bash 817 root 4r REG 0,3 0 4026532155 net
> > bash 817 root 255u CHR 136,0 0t0 3 /dev/pts/0
>
> Applied
It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
supported from 2.6.27
Not sure if its worth a patch, to do the normal socket() call followed
by legacy fcntl() one.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-07-08 14:17 ` Eric Dumazet
@ 2013-07-08 15:51 ` Andrey Wagin
2013-07-08 16:05 ` Eric Dumazet
2013-07-08 19:57 ` Stephen Hemminger
1 sibling, 1 reply; 9+ messages in thread
From: Andrey Wagin @ 2013-07-08 15:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
[-- Attachment #1: Type: text/plain, Size: 420 bytes --]
Hi Eric,
2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
>> Applied
>
> It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
> supported from 2.6.27
>
> Not sure if its worth a patch, to do the normal socket() call followed
> by legacy fcntl() one.
fcntl is not safe for multithread applications. If libnetlink isn't
going to be used for them, I don't have objection. The patch is
attached.
Thanks.
[-- Attachment #2: 0001-ip-avoid-using-SOCK_CLOEXEC-to-save-backward-compati.patch --]
[-- Type: application/octet-stream, Size: 1277 bytes --]
From 970d24097323f5d316cf160f3bc711d5f740db36 Mon Sep 17 00:00:00 2001
From: Andrey Vagin <avagin@openvz.org>
Date: Mon, 8 Jul 2013 19:08:36 +0400
Subject: [PATCH] ip: avoid using SOCK_CLOEXEC to save backward compatibility
Eric Dumazet wrote, that SOCK_CLOEXEC could break ip command for old
kernels, as SOCK_CLOEXEC is supported from 2.6.27.
BTW: fcntl(SOCK_CLOEXEC) is not safe for multithread applications.
Reported-by: <eric.dumazet@gmail.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
lib/libnetlink.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9e2a795..992aabd 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -43,12 +43,17 @@ int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions,
memset(rth, 0, sizeof(*rth));
- rth->fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, protocol);
+ rth->fd = socket(AF_NETLINK, SOCK_RAW, protocol);
if (rth->fd < 0) {
perror("Cannot open netlink socket");
return -1;
}
+ if (fcntl(rth->fd, F_SETFD, FD_CLOEXEC)) {
+ perror("Cannot set the close-on-exec flag");
+ return -1;
+ }
+
if (setsockopt(rth->fd,SOL_SOCKET,SO_SNDBUF,&sndbuf,sizeof(sndbuf)) < 0) {
perror("SO_SNDBUF");
return -1;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-07-08 15:51 ` Andrey Wagin
@ 2013-07-08 16:05 ` Eric Dumazet
2013-07-08 19:23 ` Andrey Wagin
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2013-07-08 16:05 UTC (permalink / raw)
To: Andrey Wagin; +Cc: Stephen Hemminger, netdev
On Mon, 2013-07-08 at 19:51 +0400, Andrey Wagin wrote:
> Hi Eric,
>
> 2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
> >> Applied
> >
> > It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
> > supported from 2.6.27
> >
> > Not sure if its worth a patch, to do the normal socket() call followed
> > by legacy fcntl() one.
>
> fcntl is not safe for multithread applications. If libnetlink isn't
> going to be used for them, I don't have objection. The patch is
> attached.
>
If you want multithread safety, you could attempt the SOCK_CLOEXEC, and
fall back in case of error to legacy socket()/fcntl()
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-07-08 16:05 ` Eric Dumazet
@ 2013-07-08 19:23 ` Andrey Wagin
2013-07-08 19:50 ` Stephen Hemminger
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Wagin @ 2013-07-08 19:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
[-- Attachment #1: Type: text/plain, Size: 757 bytes --]
2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
> On Mon, 2013-07-08 at 19:51 +0400, Andrey Wagin wrote:
>> Hi Eric,
>>
>> 2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
>> >> Applied
>> >
>> > It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
>> > supported from 2.6.27
>> >
>> > Not sure if its worth a patch, to do the normal socket() call followed
>> > by legacy fcntl() one.
>>
>> fcntl is not safe for multithread applications. If libnetlink isn't
>> going to be used for them, I don't have objection. The patch is
>> attached.
>>
>
> If you want multithread safety, you could attempt the SOCK_CLOEXEC, and
> fall back in case of error to legacy socket()/fcntl()
>
Yes, it's good idea. Look at the attached patch.
Thanks.
>
>
[-- Attachment #2: 0001-ip-try-to-create-socket-w-o-SOCK_CLOEXEC-if-previous.patch --]
[-- Type: application/octet-stream, Size: 1389 bytes --]
From 9e4bc17d457aded05c0cbb9c58285f15f4f1c406 Mon Sep 17 00:00:00 2001
From: Andrey Vagin <avagin@openvz.org>
Date: Mon, 8 Jul 2013 19:08:36 +0400
Subject: [PATCH] ip: try to create socket w/o SOCK_CLOEXEC, if previous
attempt failed
SOCK_CLOEXEC is supported from 2.6.27
Create socket with SOCK_CLOEXEC, if this operation failed with EINVAL,
try to create socket w/o SOCK_CLOEXEC and set SOCK_CLOEXEC with help
fcntl.
The second way is not safe for multithreaded application, but it's
better than don't close fd at all.
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
lib/libnetlink.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9e2a795..48a4da2 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -44,6 +44,16 @@ int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions,
memset(rth, 0, sizeof(*rth));
rth->fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, protocol);
+ if (rth->fd < 0 && errno == EINVAL) {
+ /* SOCK_CLOEXEC is supported from 2.6.27 */
+ rth->fd = socket(AF_NETLINK, SOCK_RAW, protocol);
+ if (rth->fd >= 0)
+ if (fcntl(rth->fd, F_SETFD, FD_CLOEXEC)) {
+ perror("Cannot set the close-on-exec flag");
+ return -1;
+ }
+ }
+
if (rth->fd < 0) {
perror("Cannot open netlink socket");
return -1;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-07-08 19:23 ` Andrey Wagin
@ 2013-07-08 19:50 ` Stephen Hemminger
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2013-07-08 19:50 UTC (permalink / raw)
To: Andrey Wagin; +Cc: Eric Dumazet, netdev
On Mon, 8 Jul 2013 23:23:35 +0400
Andrey Wagin <avagin@gmail.com> wrote:
> 2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
> > On Mon, 2013-07-08 at 19:51 +0400, Andrey Wagin wrote:
> >> Hi Eric,
> >>
> >> 2013/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
> >> >> Applied
> >> >
> >> > It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
> >> > supported from 2.6.27
> >> >
> >> > Not sure if its worth a patch, to do the normal socket() call followed
> >> > by legacy fcntl() one.
> >>
> >> fcntl is not safe for multithread applications. If libnetlink isn't
> >> going to be used for them, I don't have objection. The patch is
> >> attached.
> >>
> >
> > If you want multithread safety, you could attempt the SOCK_CLOEXEC, and
> > fall back in case of error to legacy socket()/fcntl()
> >
>
> Yes, it's good idea. Look at the attached patch.
>
> Thanks.
>
> >
> >
Just always do the fcntl(), it makes more sense rather than having
two code paths.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ip: set the close-on-exec flag for descriptors
2013-07-08 14:17 ` Eric Dumazet
2013-07-08 15:51 ` Andrey Wagin
@ 2013-07-08 19:57 ` Stephen Hemminger
2013-07-08 20:00 ` Eric Dumazet
1 sibling, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2013-07-08 19:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrey Vagin, netdev
On Mon, 08 Jul 2013 07:17:16 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2013-06-04 at 09:11 -0700, Stephen Hemminger wrote:
> > On Tue, 4 Jun 2013 12:01:14 +0400
> > Andrey Vagin <avagin@openvz.org> wrote:
> >
> > > Otherwise a program executed by "ip netns exec" has two extra
> > > descriptors.
> > >
> > > $ ip netns exec test /bin/bash
> > > $ lsof -p $$
> > > ...
> > > bash 817 root 0u CHR 136,0 0t0 3 /dev/pts/0
> > > bash 817 root 1u CHR 136,0 0t0 3 /dev/pts/0
> > > bash 817 root 2u CHR 136,0 0t0 3 /dev/pts/0
> > > bash 817 root 3u sock 0,6 0t0 13386 protocol: NETLINK
> > > bash 817 root 4r REG 0,3 0 4026532155 net
> > > bash 817 root 255u CHR 136,0 0t0 3 /dev/pts/0
> >
> > Applied
>
> It seems it could break ip command for old kernels, as SOCK_CLOEXEC is
> supported from 2.6.27
>
> Not sure if its worth a patch, to do the normal socket() call followed
> by legacy fcntl() one.
Nevermind, original patch is fine since the set namespace (NR_setns) system
call used did not show up into 3.0!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-08 20:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-04 8:01 [PATCH] ip: set the close-on-exec flag for descriptors Andrey Vagin
2013-06-04 16:11 ` Stephen Hemminger
2013-07-08 14:17 ` Eric Dumazet
2013-07-08 15:51 ` Andrey Wagin
2013-07-08 16:05 ` Eric Dumazet
2013-07-08 19:23 ` Andrey Wagin
2013-07-08 19:50 ` Stephen Hemminger
2013-07-08 19:57 ` Stephen Hemminger
2013-07-08 20:00 ` Eric Dumazet
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.