All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] two bpf tools fixes
@ 2013-12-16 10:44 Daniel Borkmann
  2013-12-16 10:45 ` [PATCH net-next 1/2] bpf_dbg: always close socket in bpf_runnable Daniel Borkmann
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Daniel Borkmann @ 2013-12-16 10:44 UTC (permalink / raw)
  To: davem; +Cc: netdev

Found two follow-up things that need to be fixed. Thanks!

Daniel Borkmann (2):
  bpf_dbg: always close socket in bpf_runnable
  bpf_exp: free duplicated labels at exit time

 tools/net/bpf_dbg.c |  2 +-
 tools/net/bpf_exp.y | 27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 8 deletions(-)

-- 
1.8.3.1

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

* [PATCH net-next 1/2] bpf_dbg: always close socket in bpf_runnable
  2013-12-16 10:44 [PATCH net-next 0/2] two bpf tools fixes Daniel Borkmann
@ 2013-12-16 10:45 ` Daniel Borkmann
  2013-12-16 10:45 ` [PATCH net-next 2/2] bpf_exp: free duplicated labels at exit time Daniel Borkmann
  2013-12-17 22:11 ` [PATCH net-next 0/2] two bpf tools fixes David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2013-12-16 10:45 UTC (permalink / raw)
  To: davem; +Cc: netdev

We must not leave the socket intact in bpf_runnable(). The socket
is used to test if the filter code is being accepted by the kernel
or not. So right after we do the setsockopt(2), we need to close
it again.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 tools/net/bpf_dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/net/bpf_dbg.c b/tools/net/bpf_dbg.c
index 0fdcb70..65dc757 100644
--- a/tools/net/bpf_dbg.c
+++ b/tools/net/bpf_dbg.c
@@ -512,11 +512,11 @@ static bool bpf_runnable(struct sock_filter *f, unsigned int len)
 		return false;
 	}
 	ret = setsockopt(sock, SOL_SOCKET, SO_ATTACH_FILTER, &bpf, sizeof(bpf));
+	close(sock);
 	if (ret < 0) {
 		rl_printf("program not allowed to run by kernel!\n");
 		return false;
 	}
-	close(sock);
 	for (i = 0; i < len; i++) {
 		if (BPF_CLASS(f[i].code) == BPF_LD &&
 		    f[i].k > SKF_AD_OFF) {
-- 
1.8.3.1

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

* [PATCH net-next 2/2] bpf_exp: free duplicated labels at exit time
  2013-12-16 10:44 [PATCH net-next 0/2] two bpf tools fixes Daniel Borkmann
  2013-12-16 10:45 ` [PATCH net-next 1/2] bpf_dbg: always close socket in bpf_runnable Daniel Borkmann
@ 2013-12-16 10:45 ` Daniel Borkmann
  2013-12-17 22:11 ` [PATCH net-next 0/2] two bpf tools fixes David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2013-12-16 10:45 UTC (permalink / raw)
  To: davem; +Cc: netdev

Valgrind found that extracted labels that are passed from the lexer
weren't freed upon exit. Therefore, add a small helper function that
walks label tables and frees them. Since also NULL can be passed to
free(3), we do not need to take care of that here. While at it, fix
up a spacing error in bpf_set_curr_label().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 tools/net/bpf_exp.y | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/tools/net/bpf_exp.y b/tools/net/bpf_exp.y
index f524110..d15efc9 100644
--- a/tools/net/bpf_exp.y
+++ b/tools/net/bpf_exp.y
@@ -40,8 +40,8 @@ extern void yyerror(const char *str);
 
 extern void bpf_asm_compile(FILE *fp, bool cstyle);
 static void bpf_set_curr_instr(uint16_t op, uint8_t jt, uint8_t jf, uint32_t k);
-static void bpf_set_curr_label(const char *label);
-static void bpf_set_jmp_label(const char *label, enum jmp_type type);
+static void bpf_set_curr_label(char *label);
+static void bpf_set_jmp_label(char *label, enum jmp_type type);
 
 %}
 
@@ -573,7 +573,7 @@ txa
 
 static int curr_instr = 0;
 static struct sock_filter out[BPF_MAXINSNS];
-static const char **labels, **labels_jt, **labels_jf, **labels_k;
+static char **labels, **labels_jt, **labels_jf, **labels_k;
 
 static void bpf_assert_max(void)
 {
@@ -594,13 +594,13 @@ static void bpf_set_curr_instr(uint16_t code, uint8_t jt, uint8_t jf,
 	curr_instr++;
 }
 
-static void bpf_set_curr_label(const char *label)
+static void bpf_set_curr_label(char *label)
 {
 	bpf_assert_max();
-        labels[curr_instr] = label;
+	labels[curr_instr] = label;
 }
 
-static void bpf_set_jmp_label(const char *label, enum jmp_type type)
+static void bpf_set_jmp_label(char *label, enum jmp_type type)
 {
 	bpf_assert_max();
 	switch (type) {
@@ -717,12 +717,25 @@ static void bpf_init(void)
 	assert(labels_k);
 }
 
+static void bpf_destroy_labels(void)
+{
+	int i;
+
+	for (i = 0; i < curr_instr; i++) {
+		free(labels_jf[i]);
+		free(labels_jt[i]);
+		free(labels_k[i]);
+		free(labels[i]);
+	}
+}
+
 static void bpf_destroy(void)
 {
-	free(labels);
+	bpf_destroy_labels();
 	free(labels_jt);
 	free(labels_jf);
 	free(labels_k);
+	free(labels);
 }
 
 void bpf_asm_compile(FILE *fp, bool cstyle)
-- 
1.8.3.1

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

* Re: [PATCH net-next 0/2] two bpf tools fixes
  2013-12-16 10:44 [PATCH net-next 0/2] two bpf tools fixes Daniel Borkmann
  2013-12-16 10:45 ` [PATCH net-next 1/2] bpf_dbg: always close socket in bpf_runnable Daniel Borkmann
  2013-12-16 10:45 ` [PATCH net-next 2/2] bpf_exp: free duplicated labels at exit time Daniel Borkmann
@ 2013-12-17 22:11 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2013-12-17 22:11 UTC (permalink / raw)
  To: dborkman; +Cc: netdev

From: Daniel Borkmann <dborkman@redhat.com>
Date: Mon, 16 Dec 2013 11:44:59 +0100

> Found two follow-up things that need to be fixed. Thanks!

Both applied, thanks Daniel.

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

end of thread, other threads:[~2013-12-17 22:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 10:44 [PATCH net-next 0/2] two bpf tools fixes Daniel Borkmann
2013-12-16 10:45 ` [PATCH net-next 1/2] bpf_dbg: always close socket in bpf_runnable Daniel Borkmann
2013-12-16 10:45 ` [PATCH net-next 2/2] bpf_exp: free duplicated labels at exit time Daniel Borkmann
2013-12-17 22:11 ` [PATCH net-next 0/2] two bpf tools fixes David Miller

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.