diff mbox

[2/2] Rule comment is now stored into a TLV attribute.

Message ID 1454614720-8126-2-git-send-email-carlosfg@riseup.net
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Carlos Falgueras García Feb. 4, 2016, 7:38 p.m. UTC 
Modify the rule structure and the parser to store the comment string into a TLV
attribute. This way more data of any type could be stored with a rule.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 include/rule.h            |  4 +++-
 src/netlink_delinearize.c | 10 ++++++++--
 src/netlink_linearize.c   |  4 +++-
 src/parser_bison.y        | 19 ++++++++++++++++---
 src/rule.c                | 12 +++++++++---
 5 files changed, 39 insertions(+), 10 deletions(-)

Comments

Arturo Borrero Feb. 5, 2016, 7:30 a.m. UTC | #1 
On 4 February 2016 at 20:38, Carlos Falgueras García
<carlosfg@riseup.net> wrote:
> Modify the rule structure and the parser to store the comment string into a TLV
> attribute. This way more data of any type could be stored with a rule.
>
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  include/rule.h            |  4 +++-
>  src/netlink_delinearize.c | 10 ++++++++--
>  src/netlink_linearize.c   |  4 +++-
>  src/parser_bison.y        | 19 ++++++++++++++++---
>  src/rule.c                | 12 +++++++++---
>  5 files changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/include/rule.h b/include/rule.h
> index c848f0f..7a18c50 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -163,9 +163,11 @@ struct rule {
>         struct location         location;
>         struct list_head        stmts;
>         unsigned int            num_stmts;
> -       const char              *comment;
> +       struct nftnl_attrbuf    *comment;
>  };
>
> +#define RULE_COMMENT_MAX_LEN 256
> +
>  extern struct rule *rule_alloc(const struct location *loc,
>                                const struct handle *h);
>  extern void rule_free(struct rule *rule);
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index ae6abb0..d0d85aa 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -25,6 +25,7 @@
>  #include <utils.h>
>  #include <erec.h>
>  #include <sys/socket.h>
> +#include <libnftnl/attr.h>
>
>  struct netlink_parse_ctx {
>         struct list_head        *msgs;
> @@ -1734,12 +1735,17 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
>         assert(pctx->table != NULL);
>
>         if (nftnl_rule_is_set(nlr, NFTNL_RULE_USERDATA)) {
> +               struct nftnl_attrbuf *attrbuf;
>                 const void *data;
>                 uint32_t len;
>
>                 data = nftnl_rule_get_data(nlr, NFTNL_RULE_USERDATA, &len);
> -               pctx->rule->comment = xmalloc(len);
> -               memcpy((char *)pctx->rule->comment, data, len);
> +               if (!(attrbuf = nftnl_attrbuf_alloc(len))) {

If I understand correctly above, you alloc here with length 'len'.

> +                       perror("Error allocating memory for attrbuf: ");

Minor thing here regarding perror: from the manpage <<the argument
string is printed, followed by a colon and a blank>>. So you would be
printing something like 'attrbuf: : '

> +                       exit(EXIT_FAILURE);
> +               }
> +               pctx->rule->comment = nftnl_attrbuf_alloc(len);

Another alloc above with length 'len', memleak warning.

> +               nftnl_attrbuf_set_data(pctx->rule->comment, data, len);
>         }
>
>         nftnl_expr_foreach((struct nftnl_rule *)nlr, netlink_parse_expr, pctx);
Carlos Falgueras García Feb. 5, 2016, 2:32 p.m. UTC | #2 
Thanks for the corrections. I revised it and realized I must fix many 
other things. Sorry for that. Soon I will send a new version, fixed and 
reviewed in depth.
--
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
diff mbox

Patch

diff --git a/include/rule.h b/include/rule.h
index c848f0f..7a18c50 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -163,9 +163,11 @@  struct rule {
 	struct location		location;
 	struct list_head	stmts;
 	unsigned int		num_stmts;
-	const char		*comment;
+	struct nftnl_attrbuf	*comment;
 };
 
+#define RULE_COMMENT_MAX_LEN 256
+
 extern struct rule *rule_alloc(const struct location *loc,
 			       const struct handle *h);
 extern void rule_free(struct rule *rule);
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index ae6abb0..d0d85aa 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -25,6 +25,7 @@ 
 #include <utils.h>
 #include <erec.h>
 #include <sys/socket.h>
+#include <libnftnl/attr.h>
 
 struct netlink_parse_ctx {
 	struct list_head	*msgs;
@@ -1734,12 +1735,17 @@  struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 	assert(pctx->table != NULL);
 
 	if (nftnl_rule_is_set(nlr, NFTNL_RULE_USERDATA)) {
+		struct nftnl_attrbuf *attrbuf;
 		const void *data;
 		uint32_t len;
 
 		data = nftnl_rule_get_data(nlr, NFTNL_RULE_USERDATA, &len);
-		pctx->rule->comment = xmalloc(len);
-		memcpy((char *)pctx->rule->comment, data, len);
+		if (!(attrbuf = nftnl_attrbuf_alloc(len))) {
+			perror("Error allocating memory for attrbuf: ");
+			exit(EXIT_FAILURE);
+		}
+		pctx->rule->comment = nftnl_attrbuf_alloc(len);
+		nftnl_attrbuf_set_data(pctx->rule->comment, data, len);
 	}
 
 	nftnl_expr_foreach((struct nftnl_rule *)nlr, netlink_parse_expr, pctx);
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 86b49c6..4fd5e1e 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -21,6 +21,7 @@ 
 #include <netinet/in.h>
 
 #include <linux/netfilter.h>
+#include <libnftnl/attr.h>
 
 
 struct netlink_linearize_ctx {
@@ -1110,7 +1111,8 @@  void netlink_linearize_rule(struct netlink_ctx *ctx, struct nftnl_rule *nlr,
 
 	if (rule->comment)
 		nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA,
-				    rule->comment, strlen(rule->comment) + 1);
+				    nftnl_attrbuf_get_data(rule->comment),
+				    nftnl_attrbuf_get_len(rule->comment));
 
 	netlink_dump_rule(nlr);
 }
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 05ade0f..3ebed7a 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -24,6 +24,7 @@ 
 #include <netinet/icmp6.h>
 #include <libnftnl/common.h>
 #include <libnftnl/set.h>
+#include <libnftnl/attr.h>
 
 #include <rule.h>
 #include <statement.h>
@@ -132,6 +133,7 @@  static void location_update(struct location *loc, struct location *rhs, int n)
 	struct stmt		*stmt;
 	struct expr		*expr;
 	struct set		*set;
+	struct nftnl_attrbuf	*attrbuf;
 	const struct datatype	*datatype;
 }
 
@@ -406,8 +408,8 @@  static void location_update(struct location *loc, struct location *rhs, int n)
 %token XML			"xml"
 %token JSON			"json"
 
-%type <string>			identifier type_identifier string comment_spec
-%destructor { xfree($$); }	identifier type_identifier string comment_spec
+%type <string>			identifier type_identifier string
+%destructor { xfree($$); }	identifier type_identifier string
 
 %type <val>			time_spec
 
@@ -432,6 +434,8 @@  static void location_update(struct location *loc, struct location *rhs, int n)
 %destructor { close_scope(state); chain_free($$); }	chain_block_alloc
 %type <rule>			rule
 %destructor { rule_free($$); }	rule
+%type <attrbuf>			comment_spec
+%destructor { nftnl_attrbuf_free($$); }      comment_spec
 
 %type <val>			set_flag_list	set_flag
 
@@ -1283,7 +1287,16 @@  comment_spec		:	/* empty */
 			}
 			|	COMMENT		string
 			{
-				$$ = $2;
+				$$ = nftnl_attrbuf_alloc(RULE_COMMENT_MAX_LEN);
+				if (!nftnl_attr_put_check($$,
+					RULE_COMMENT_MAX_LEN,
+					NFTNL_ATTR_TYPE_STRING,
+					strlen($2), $2)
+				) {
+					erec_queue(error(&@2, "Comment too long: \"%s\"", $2),
+						state->msgs);
+					YYERROR;
+				}
 			}
 			;
 
diff --git a/src/rule.c b/src/rule.c
index 18ff592..79c665c 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -23,6 +23,7 @@ 
 
 #include <libnftnl/common.h>
 #include <libnftnl/ruleset.h>
+#include <libnftnl/attr.h>
 #include <netinet/ip.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter_arp.h>
@@ -368,6 +369,7 @@  struct rule *rule_alloc(const struct location *loc, const struct handle *h)
 	init_list_head(&rule->stmts);
 	if (h != NULL)
 		rule->handle = *h;
+	rule->comment = NULL;
 	return rule;
 }
 
@@ -375,21 +377,25 @@  void rule_free(struct rule *rule)
 {
 	stmt_list_free(&rule->stmts);
 	handle_free(&rule->handle);
-	xfree(rule->comment);
+	if (rule->comment)
+		nftnl_attrbuf_free(rule->comment);
 	xfree(rule);
 }
 
 void rule_print(const struct rule *rule)
 {
 	const struct stmt *stmt;
+	struct nftnl_attr *attr;
 
 	list_for_each_entry(stmt, &rule->stmts, list) {
 		stmt->ops->print(stmt);
 		printf(" ");
 	}
 
-	if (rule->comment)
-		printf("comment \"%s\" ", rule->comment);
+	if (rule->comment) {
+		attr = nftnl_attrbuf_get_head(rule->comment);
+		printf("comment \"%s\" ", (char *)nftnl_attr_get_value(attr));
+	}
 
 	if (handle_output > 0)
 		printf("# handle %" PRIu64, rule->handle.handle);