netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netlink: do not SIGSEGV when socket() fails
@ 2013-12-04 19:12 Shawn Landden
  2013-12-04 19:30 ` Arturo Borrero Gonzalez
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn Landden @ 2013-12-04 19:12 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Shawn Landden

---
 src/netlink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 533634a..664487d 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -33,8 +33,11 @@ static struct mnl_socket *nf_sock;
 static void __init netlink_open_sock(void)
 {
 	nf_sock = mnl_socket_open(NETLINK_NETFILTER);
-	if (nf_sock == NULL)
+	if (nf_sock == NULL) {
+                fprintf(fdopen(STDERR_FILENO, "a"), "Could not open AF_NETLINK socket: %s\n",
+                                        strerror(errno));
 		memory_allocation_error();
+        }
 
 	fcntl(mnl_socket_get_fd(nf_sock), F_SETFL, O_NONBLOCK);
 	mnl_batch_init();
@@ -42,7 +45,8 @@ static void __init netlink_open_sock(void)
 
 static void __exit netlink_close_sock(void)
 {
-	mnl_socket_close(nf_sock);
+        if (nf_sock)
+                mnl_socket_close(nf_sock);
 }
 
 int netlink_io_error(struct netlink_ctx *ctx, const struct location *loc,
-- 
1.8.5.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] netlink: do not SIGSEGV when socket() fails
  2013-12-04 19:12 [PATCH] netlink: do not SIGSEGV when socket() fails Shawn Landden
@ 2013-12-04 19:30 ` Arturo Borrero Gonzalez
  2013-12-04 20:01   ` [nftables PATCH] " Shawn Landden
  0 siblings, 1 reply; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2013-12-04 19:30 UTC (permalink / raw)
  To: Shawn Landden; +Cc: Netfilter Development Mailing list

On 4 December 2013 20:12, Shawn Landden <shawn@churchofgit.com> wrote:
> ---
>  src/netlink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>

Hi!

I would suggest to provide a description and a Signed-off-by line
within the patch.
Also, a small trace may help others (like me) to understand the situation.

> diff --git a/src/netlink.c b/src/netlink.c
> index 533634a..664487d 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -33,8 +33,11 @@ static struct mnl_socket *nf_sock;
>  static void __init netlink_open_sock(void)
>  {
>         nf_sock = mnl_socket_open(NETLINK_NETFILTER);
> -       if (nf_sock == NULL)
> +       if (nf_sock == NULL) {
> +                fprintf(fdopen(STDERR_FILENO, "a"), "Could not open AF_NETLINK socket: %s\n",
> +                                        strerror(errno));

In your patch, the code seem indented with spaces. Tabs are used all
around the nft code.
Also, the second line should be aligned with the position+1 of first
'(' character of fprintf.

bonus: please consider adding a tag to the [PATCH] subject, like [nft
PATCH] or [libmnl PATCH].

regards.
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [nftables PATCH] netlink: do not SIGSEGV when socket() fails
  2013-12-04 19:30 ` Arturo Borrero Gonzalez
@ 2013-12-04 20:01   ` Shawn Landden
  2013-12-04 20:30     ` Florian Westphal
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn Landden @ 2013-12-04 20:01 UTC (permalink / raw)
  To: netfilter-devel; +Cc: arturo.borrero.glez, Shawn Landden

Program received signal SIGSEGV, Segmentation fault.
mnl_socket_close (nl=0x0) at socket.c:248
248        int ret = close(nl->fd);
(gdb) bt
 #0  mnl_socket_close (nl=0x0) at socket.c:248
 #1  0x0000000000410f79 in netlink_close_sock () at src/netlink.c:45
 #2  0x00007ffff7de9fcf in _dl_fini () at dl-fini.c:253
 #3  0x00007ffff717fa91 in __run_exit_handlers (status=2, listp=0x7ffff74ec5c8
<__exit_funcs>,
    run_list_atexit=run_list_atexit@entry=true) at exit.c:77
 #4  0x00007ffff717fb15 in __GI_exit (status=<optimized out>) at exit.c:99
 #5  0x0000000000419b60 in memory_allocation_error () at src/utils.c:24
 #6  0x0000000000410f3a in netlink_open_sock () at src/netlink.c:37
 #7  0x00000000004291cd in __libc_csu_init ()
 #8  0x00007ffff7167925 in __libc_start_main (main=0x405219 <main>, argc=1,
    ubp_av=0x7fffffffe738, init=0x429170 <__libc_csu_init>, fini=<optimized
out>,
    rtld_fini=<optimized out>, stack_end=0x7fffffffe728) at libc-start.c:235
 #9  0x0000000000404d49 in _start ()

Signed-off-by: Shawn Landden <shawn@churchofgit.com>
---
 src/netlink.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 533634a..5b2961e 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -33,8 +33,14 @@ static struct mnl_socket *nf_sock;
 static void __init netlink_open_sock(void)
 {
 	nf_sock = mnl_socket_open(NETLINK_NETFILTER);
-	if (nf_sock == NULL)
-		memory_allocation_error();
+	if (nf_sock == NULL) {
+		fprintf(fdopen(STDERR_FILENO, "a"),
+			"Could not open AF_NETLINK socket: %s\n",
+			strerror(errno));
+		/* the __exit netlink_close_sock() will dereference nf_sock
+		   if we exit() */
+		_exit(EFT_EXIT_NOMEM);
+	}
 
 	fcntl(mnl_socket_get_fd(nf_sock), F_SETFL, O_NONBLOCK);
 	mnl_batch_init();
-- 
1.8.5.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [nftables PATCH] netlink: do not SIGSEGV when socket() fails
  2013-12-04 20:01   ` [nftables PATCH] " Shawn Landden
@ 2013-12-04 20:30     ` Florian Westphal
  2013-12-05  3:18       ` [PATCH] " Shawn Landden
  0 siblings, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2013-12-04 20:30 UTC (permalink / raw)
  To: Shawn Landden; +Cc: netfilter-devel, arturo.borrero.glez

Shawn Landden <shawn@churchofgit.com> wrote:
> +	if (nf_sock == NULL) {
> +		fprintf(fdopen(STDERR_FILENO, "a"),
> +			"Could not open AF_NETLINK socket: %s\n",
> +			strerror(errno));

Why this fdopen() [ which can fail ] construct?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] netlink: do not SIGSEGV when socket() fails
  2013-12-04 20:30     ` Florian Westphal
@ 2013-12-05  3:18       ` Shawn Landden
  2013-12-05  9:03         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn Landden @ 2013-12-05  3:18 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Shawn Landden

Program received signal SIGSEGV, Segmentation fault.
mnl_socket_close (nl=0x0) at socket.c:248
248        int ret = close(nl->fd);
(gdb) bt
 #0  mnl_socket_close (nl=0x0) at socket.c:248
 #1  0x0000000000410f79 in netlink_close_sock () at src/netlink.c:45
 #2  0x00007ffff7de9fcf in _dl_fini () at dl-fini.c:253
 #3  0x00007ffff717fa91 in __run_exit_handlers (status=2, listp=0x7ffff74ec5c8
<__exit_funcs>,
    run_list_atexit=run_list_atexit@entry=true) at exit.c:77
 #4  0x00007ffff717fb15 in __GI_exit (status=<optimized out>) at exit.c:99
 #5  0x0000000000419b60 in memory_allocation_error () at src/utils.c:24
 #6  0x0000000000410f3a in netlink_open_sock () at src/netlink.c:37
 #7  0x00000000004291cd in __libc_csu_init ()
 #8  0x00007ffff7167925 in __libc_start_main (main=0x405219 <main>, argc=1,
    ubp_av=0x7fffffffe738, init=0x429170 <__libc_csu_init>, fini=<optimized
out>,
    rtld_fini=<optimized out>, stack_end=0x7fffffffe728) at libc-start.c:235
 #9  0x0000000000404d49 in _start ()

The include of <nftables.h> is wierd so I can't use NFT_EXIT_NOMEM in this file.

Signed-off-by: Shawn Landden <shawn@churchofgit.com>
---
 src/netlink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 533634a..5240633 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -33,8 +33,11 @@ static struct mnl_socket *nf_sock;
 static void __init netlink_open_sock(void)
 {
 	nf_sock = mnl_socket_open(NETLINK_NETFILTER);
-	if (nf_sock == NULL)
+	if (nf_sock == NULL) {
+		dprintf(STDERR_FILENO,
+			"Could not open AF_NETLINK socket: %m\n");
 		memory_allocation_error();
+	}
 
 	fcntl(mnl_socket_get_fd(nf_sock), F_SETFL, O_NONBLOCK);
 	mnl_batch_init();
@@ -42,7 +45,8 @@ static void __init netlink_open_sock(void)
 
 static void __exit netlink_close_sock(void)
 {
-	mnl_socket_close(nf_sock);
+	if (nf_sock)
+		mnl_socket_close(nf_sock);
 }
 
 int netlink_io_error(struct netlink_ctx *ctx, const struct location *loc,
-- 
1.8.5.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] netlink: do not SIGSEGV when socket() fails
  2013-12-05  3:18       ` [PATCH] " Shawn Landden
@ 2013-12-05  9:03         ` Pablo Neira Ayuso
  0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2013-12-05  9:03 UTC (permalink / raw)
  To: Shawn Landden; +Cc: netfilter-devel

On Wed, Dec 04, 2013 at 07:18:53PM -0800, Shawn Landden wrote:
> Program received signal SIGSEGV, Segmentation fault.
> mnl_socket_close (nl=0x0) at socket.c:248
> 248        int ret = close(nl->fd);
> (gdb) bt
>  #0  mnl_socket_close (nl=0x0) at socket.c:248
>  #1  0x0000000000410f79 in netlink_close_sock () at src/netlink.c:45
>  #2  0x00007ffff7de9fcf in _dl_fini () at dl-fini.c:253
>  #3  0x00007ffff717fa91 in __run_exit_handlers (status=2, listp=0x7ffff74ec5c8
> <__exit_funcs>,
>     run_list_atexit=run_list_atexit@entry=true) at exit.c:77
>  #4  0x00007ffff717fb15 in __GI_exit (status=<optimized out>) at exit.c:99
>  #5  0x0000000000419b60 in memory_allocation_error () at src/utils.c:24
>  #6  0x0000000000410f3a in netlink_open_sock () at src/netlink.c:37
>  #7  0x00000000004291cd in __libc_csu_init ()
>  #8  0x00007ffff7167925 in __libc_start_main (main=0x405219 <main>, argc=1,
>     ubp_av=0x7fffffffe738, init=0x429170 <__libc_csu_init>, fini=<optimized
> out>,
>     rtld_fini=<optimized out>, stack_end=0x7fffffffe728) at libc-start.c:235
>  #9  0x0000000000404d49 in _start ()
> 
> The include of <nftables.h> is wierd so I can't use NFT_EXIT_NOMEM in this file.

Please, add NFT_EXIT_NONL.

> Signed-off-by: Shawn Landden <shawn@churchofgit.com>
> ---
>  src/netlink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/netlink.c b/src/netlink.c
> index 533634a..5240633 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -33,8 +33,11 @@ static struct mnl_socket *nf_sock;
>  static void __init netlink_open_sock(void)
>  {
>  	nf_sock = mnl_socket_open(NETLINK_NETFILTER);
> -	if (nf_sock == NULL)
> +	if (nf_sock == NULL) {
> +		dprintf(STDERR_FILENO,
> +			"Could not open AF_NETLINK socket: %m\n");
>  		memory_allocation_error();
> +	}

Better add this function to utils.c

void __noreturn netlink_error(void)
{
        fprintf(stderr, "Netlink failure: %s\n", strerror(errno));
        exit(NFT_EXIT_NONL);
}

And call it from there. Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-12-05  9:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-04 19:12 [PATCH] netlink: do not SIGSEGV when socket() fails Shawn Landden
2013-12-04 19:30 ` Arturo Borrero Gonzalez
2013-12-04 20:01   ` [nftables PATCH] " Shawn Landden
2013-12-04 20:30     ` Florian Westphal
2013-12-05  3:18       ` [PATCH] " Shawn Landden
2013-12-05  9:03         ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).