[PATCH 0/6] conntrack: fixes for handling unknown protocols

[PATCH 0/6] conntrack: fixes for handling unknown protocols

[Mikhail Sennikovsky]

Hi Pablo & all,

Here is a set of fixes and test cases for handling ct entries of
protocols unknown to the conntrack tool.
The six commits are organized in a way: a commit with a testcase
showing an issue, followed by a commit fixing the issue.

Comments/suggestions are very welcome as always.

Regards,
Mikhail

Mikhail Sennikovsky (6):
  tests/conntrack: ct create for unknown protocols
  conntrack: set reply l4 proto for unknown protocol
  tests/conntrack: invalid protocol values
  conntrack: fix protocol number parsing
  tests/conntrack: ct -o save for unknown protocols
  conntrack: fix -o save dump for unknown protocols

 extensions/libct_proto_unknown.c    | 11 ++++++++++
 src/conntrack.c                     | 33 ++++++++++++++++++++++++++---
 tests/conntrack/testsuite/00create  | 32 ++++++++++++++++++++++++++++
 tests/conntrack/testsuite/09dumpopt | 26 +++++++++++++++++++++++
 4 files changed, 99 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/6] tests/conntrack: ct create for unknown protocols

[Mikhail Sennikovsky]

Testcases covering creation of ct entries of unknown L4 protocols,
which does not work properly at the moment.
Fix included in the next commit.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 tests/conntrack/testsuite/00create | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
index 911e711..9962e23 100644
--- a/tests/conntrack/testsuite/00create
+++ b/tests/conntrack/testsuite/00create
@@ -34,3 +34,30 @@
 -I -t 29 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
 # delete icmp ping request entry
 -D -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
+# Test protocols unknown by the conntrack tool
+# IGMP
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Create again - should fail
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; BAD
+# repeat using protocol name instead of the value, should fail as well
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; BAD
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete again should fail
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; BAD
+# create using protocol name instead of the value
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; OK
+# update
+-U -t 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# delete again should fail
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p igmp ; BAD
+# take some protocol that is not normally not in /etc/protocols
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# update
+-U -t 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# delete
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# delete again
+-D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
-- 
2.25.1

[PATCH 2/6] conntrack: set reply l4 proto for unknown protocol

[Mikhail Sennikovsky]

Withouth reply l4 protocol being set consistently the mnl_cb_run
(in fact the kernel) would return EINVAL.

Make sure the reply l4 protocol is set properly for unknown
protocols.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 extensions/libct_proto_unknown.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/extensions/libct_proto_unknown.c b/extensions/libct_proto_unknown.c
index 2a47704..992b1ed 100644
--- a/extensions/libct_proto_unknown.c
+++ b/extensions/libct_proto_unknown.c
@@ -21,10 +21,21 @@ static void help(void)
 	fprintf(stdout, "  no options (unsupported)\n");
 }
 
+static void final_check(unsigned int flags,
+		        unsigned int cmd,
+		        struct nf_conntrack *ct)
+{
+	if (nfct_attr_is_set(ct, ATTR_REPL_L3PROTO) &&
+	    nfct_attr_is_set(ct, ATTR_L4PROTO) &&
+	    !nfct_attr_is_set(ct, ATTR_REPL_L4PROTO))
+		nfct_set_attr_u8(ct, ATTR_REPL_L4PROTO, nfct_get_attr_u8(ct, ATTR_L4PROTO));
+}
+
 struct ctproto_handler ct_proto_unknown = {
 	.name 		= "unknown",
 	.help		= help,
 	.opts		= opts,
+	.final_check = final_check,
 	.version	= VERSION,
 };
 
-- 
2.25.1

Re: [PATCH 2/6] conntrack: set reply l4 proto for unknown protocol

[Pablo Neira Ayuso]

On Thu, Jun 23, 2022 at 07:49:56PM +0200, Mikhail Sennikovsky wrote:
> Withouth reply l4 protocol being set consistently the mnl_cb_run
> (in fact the kernel) would return EINVAL.
> 
> Make sure the reply l4 protocol is set properly for unknown
> protocols.
> 
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
>  extensions/libct_proto_unknown.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/extensions/libct_proto_unknown.c b/extensions/libct_proto_unknown.c
> index 2a47704..992b1ed 100644
> --- a/extensions/libct_proto_unknown.c
> +++ b/extensions/libct_proto_unknown.c
> @@ -21,10 +21,21 @@ static void help(void)
>  	fprintf(stdout, "  no options (unsupported)\n");
>  }
>  
> +static void final_check(unsigned int flags,
> +		        unsigned int cmd,
> +		        struct nf_conntrack *ct)
> +{
> +	if (nfct_attr_is_set(ct, ATTR_REPL_L3PROTO) &&
> +	    nfct_attr_is_set(ct, ATTR_L4PROTO) &&
> +	    !nfct_attr_is_set(ct, ATTR_REPL_L4PROTO))
> +		nfct_set_attr_u8(ct, ATTR_REPL_L4PROTO, nfct_get_attr_u8(ct, ATTR_L4PROTO));
> +}
> +
>  struct ctproto_handler ct_proto_unknown = {
>  	.name 		= "unknown",
>  	.help		= help,
>  	.opts		= opts,
> +	.final_check = final_check,

missing indent to align it with other C99 initializers (coding style nitpick)

	.final_check    = final_check,

>  	.version	= VERSION,
>  };
>  
> -- 
> 2.25.1
> 

[PATCH 3/6] tests/conntrack: invalid protocol values

[Mikhail Sennikovsky]

Testcases covering passing invalid protocol values via -p parameter.
* -p 256 should fail
* -p foo should fail
which does not work properly at the moment.
Fix included in the next commit.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 tests/conntrack/testsuite/00create | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
index 9962e23..9fb3a0b 100644
--- a/tests/conntrack/testsuite/00create
+++ b/tests/conntrack/testsuite/00create
@@ -61,3 +61,8 @@
 -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
 # delete again
 -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
+# Invalid protocol values
+# 256 should fail
+-I -t 10 -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p 256 ; BAD
+# take some invalid protocol name
+-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p foo ; BAD
-- 
2.25.1

Re: [PATCH 3/6] tests/conntrack: invalid protocol values

[Pablo Neira Ayuso]

On Thu, Jun 23, 2022 at 07:49:57PM +0200, Mikhail Sennikovsky wrote:
> Testcases covering passing invalid protocol values via -p parameter.
> * -p 256 should fail
> * -p foo should fail
> which does not work properly at the moment.
> Fix included in the next commit.

Please, collapse this to next patch 4/6 in v2.

> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
>  tests/conntrack/testsuite/00create | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/conntrack/testsuite/00create b/tests/conntrack/testsuite/00create
> index 9962e23..9fb3a0b 100644
> --- a/tests/conntrack/testsuite/00create
> +++ b/tests/conntrack/testsuite/00create
> @@ -61,3 +61,8 @@
>  -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
>  # delete again
>  -D -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; BAD
> +# Invalid protocol values
> +# 256 should fail
> +-I -t 10 -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p 256 ; BAD
> +# take some invalid protocol name
> +-I -t 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p foo ; BAD
> -- 
> 2.25.1
> 

[PATCH 4/6] conntrack: fix protocol number parsing

[Mikhail Sennikovsky]

Before this commit it was possible to successfully create a ct entry
passing -p 256 and -p some_nonsense.
In both cases an entry with the protocol=0 would be created.

Do not allow invalid protocol values to -p option.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 src/conntrack.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index 500e736..dca7da6 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -882,6 +882,24 @@ static int ct_save_snprintf(char *buf, size_t len,
 
 extern struct ctproto_handler ct_proto_unknown;
 
+static int parse_proto_num(const char *str)
+{
+	char *endptr;
+	long val;
+
+	errno = 0;
+	val = strtol(str, &endptr, 0);
+	if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
+	    (errno != 0 && val == 0) ||
+	    endptr == str ||
+	    *endptr != '\0' ||
+	    val >= IPPROTO_MAX) {
+		return -1;
+	}
+
+	return val;
+}
+
 static struct ctproto_handler *findproto(char *name, int *pnum)
 {
 	struct ctproto_handler *cur;
@@ -901,8 +919,8 @@ static struct ctproto_handler *findproto(char *name, int *pnum)
 		return &ct_proto_unknown;
 	}
 	/* using a protocol number? */
-	protonum = atoi(name);
-	if (protonum >= 0 && protonum <= IPPROTO_MAX) {
+	protonum = parse_proto_num(name);
+	if (protonum >= 0) {
 		/* try lookup by number, perhaps this protocol is supported */
 		list_for_each_entry(cur, &proto_list, head) {
 			if (cur->protonum == protonum) {
-- 
2.25.1

Re: [PATCH 4/6] conntrack: fix protocol number parsing

[Pablo Neira Ayuso]

On Thu, Jun 23, 2022 at 07:49:58PM +0200, Mikhail Sennikovsky wrote:
> Before this commit it was possible to successfully create a ct entry
> passing -p 256 and -p some_nonsense.
> In both cases an entry with the protocol=0 would be created.
> 
> Do not allow invalid protocol values to -p option.
> 
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
>  src/conntrack.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conntrack.c b/src/conntrack.c
> index 500e736..dca7da6 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -882,6 +882,24 @@ static int ct_save_snprintf(char *buf, size_t len,
>  
>  extern struct ctproto_handler ct_proto_unknown;
>  
> +static int parse_proto_num(const char *str)
> +{
> +	char *endptr;
> +	long val;
> +
> +	errno = 0;
> +	val = strtol(str, &endptr, 0);
> +	if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
> +	    (errno != 0 && val == 0) ||
> +	    endptr == str ||
> +	    *endptr != '\0' ||
> +	    val >= IPPROTO_MAX) {

There might be a more simple way to do error reporting for strtoul?

> +		return -1;
> +	}
> +
> +	return val;
> +}
> +
>  static struct ctproto_handler *findproto(char *name, int *pnum)
>  {
>  	struct ctproto_handler *cur;
> @@ -901,8 +919,8 @@ static struct ctproto_handler *findproto(char *name, int *pnum)
>  		return &ct_proto_unknown;
>  	}
>  	/* using a protocol number? */
> -	protonum = atoi(name);
> -	if (protonum >= 0 && protonum <= IPPROTO_MAX) {
> +	protonum = parse_proto_num(name);
> +	if (protonum >= 0) {
>  		/* try lookup by number, perhaps this protocol is supported */
>  		list_for_each_entry(cur, &proto_list, head) {
>  			if (cur->protonum == protonum) {
> -- 
> 2.25.1
> 

[PATCH 5/6] tests/conntrack: ct -o save for unknown protocols

[Mikhail Sennikovsky]

Testcases covering dumping in -o save formate of ct entries of
L4 protocols unknown to the conntrack tool,
which does not work properly at the moment.
Fix included in the next commit.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 tests/conntrack/testsuite/09dumpopt | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
index 447590b..c1e0e6e 100644
--- a/tests/conntrack/testsuite/09dumpopt
+++ b/tests/conntrack/testsuite/09dumpopt
@@ -145,3 +145,29 @@
 -D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
 # clean up after yourself
 -D -w 10 ; OK
+# Cover protocols unknown to the conntrack tool
+# Create a conntrack entries
+# IGMP
+-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ;
+# Some fency protocol
+-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ;
+# Some fency protocol with IPv6
+-I -w 10 -t 59 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ;
+-R - ; OK
+# copy to zone 11
+-L -w 10 -o save ; |s/-w 10/-w 11/g
+-R - ; OK
+# Delete stuff in zone 10, should succeed
+# IGMP
+-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Some fency protocol
+-D -w 10  -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# Some fency protocol with IPv6
+-D -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
+# Delete stuff in zone 11, should succeed
+# IGMP
+-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
+# Some fency protocol
+-D -w 11  -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
+# Some fency protocol with IPv6
+-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
-- 
2.25.1

Re: [PATCH 5/6] tests/conntrack: ct -o save for unknown protocols

[Pablo Neira Ayuso]

On Thu, Jun 23, 2022 at 07:49:59PM +0200, Mikhail Sennikovsky wrote:
> Testcases covering dumping in -o save formate of ct entries of
> L4 protocols unknown to the conntrack tool,
> which does not work properly at the moment.
> Fix included in the next commit.

Could you collapse the test to the relevant patch (next commit as
description suggests)? I find this easier to find when looking at git
annotate.

> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
>  tests/conntrack/testsuite/09dumpopt | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tests/conntrack/testsuite/09dumpopt b/tests/conntrack/testsuite/09dumpopt
> index 447590b..c1e0e6e 100644
> --- a/tests/conntrack/testsuite/09dumpopt
> +++ b/tests/conntrack/testsuite/09dumpopt
> @@ -145,3 +145,29 @@
>  -D -w 11 -u SEEN_REPLY -s 1.1.1.1 -d 2.2.2.2 -r 2.2.2.2 -q 1.1.1.1 -p icmp --icmp-type 8 --icmp-code 0 --icmp-id 1226 ; OK
>  # clean up after yourself
>  -D -w 10 ; OK
> +# Cover protocols unknown to the conntrack tool
> +# Create a conntrack entries
> +# IGMP
> +-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ;
> +# Some fency protocol
> +-I -w 10 -t 59 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ;
> +# Some fency protocol with IPv6
> +-I -w 10 -t 59 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ;
> +-R - ; OK
> +# copy to zone 11
> +-L -w 10 -o save ; |s/-w 10/-w 11/g
> +-R - ; OK
> +# Delete stuff in zone 10, should succeed
> +# IGMP
> +-D -w 10 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
> +# Some fency protocol
> +-D -w 10  -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
> +# Some fency protocol with IPv6
> +-D -w 10 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
> +# Delete stuff in zone 11, should succeed
> +# IGMP
> +-D -w 11 -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 2 ; OK
> +# Some fency protocol
> +-D -w 11  -s 0.0.0.0 -d 224.0.0.22 -r 224.0.0.22 -q 0.0.0.0 -p 200 ; OK
> +# Some fency protocol with IPv6
> +-D -w 11 -s 2001:DB8::1.1.1.1 -d 2001:DB8::2.2.2.2 -p 200 ; OK
> -- 
> 2.25.1
> 

[PATCH 6/6] conntrack: fix -o save dump for unknown protocols

[Mikhail Sennikovsky]

Make sure the protocol (-p) option is included in the -o save
ct entry dumps for L4 protocols unknown to the conntrack tool

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
---
 src/conntrack.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index dca7da6..f8a228f 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -870,9 +870,18 @@ static int ct_save_snprintf(char *buf, size_t len,
 
 		ret = ct_snprintf_opts(buf + offset, len, ct, cur->print_opts);
 		BUFFER_SIZE(ret, size, len, offset);
-		break;
+		goto done_proto4;
 	}
 
+	/**
+	 * Do not use getprotobynumber here to ensure
+	 * "-o save" data incompatibility between hosts having
+	 * different /etc/protocols contents
+	 */
+	ret = snprintf(buf + offset, len, "-p %d ", l4proto);
+	BUFFER_SIZE(ret, size, len, offset);
+
+done_proto4:
 	/* skip trailing space, if any */
 	for (; size && buf[size-1] == ' '; --size)
 		buf[size-1] = '\0';
-- 
2.25.1

Re: [PATCH 6/6] conntrack: fix -o save dump for unknown protocols

[Pablo Neira Ayuso]

On Thu, Jun 23, 2022 at 07:50:00PM +0200, Mikhail Sennikovsky wrote:
> Make sure the protocol (-p) option is included in the -o save
> ct entry dumps for L4 protocols unknown to the conntrack tool
> 
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com>
> ---
>  src/conntrack.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conntrack.c b/src/conntrack.c
> index dca7da6..f8a228f 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -870,9 +870,18 @@ static int ct_save_snprintf(char *buf, size_t len,
>  
>  		ret = ct_snprintf_opts(buf + offset, len, ct, cur->print_opts);
>  		BUFFER_SIZE(ret, size, len, offset);
> -		break;
> +		goto done_proto4;

I'd suggest:

                l4proto_set = true;

so you can remove this goto.

>  	}
>  
> +	/**
> +	 * Do not use getprotobynumber here to ensure
> +	 * "-o save" data incompatibility between hosts having
> +	 * different /etc/protocols contents
> +	 */

No need for this comment, explain this in the commit message, git
annotate will help to find the reason for this.

> +	ret = snprintf(buf + offset, len, "-p %d ", l4proto);
> +	BUFFER_SIZE(ret, size, len, offset);

        if (!l4proto_set) {
                ret = snprintf(buf + offset, len, "-p %d ", l4proto);
                BUFFER_SIZE(ret, size, len, offset);
        }

> +
> +done_proto4:
>  	/* skip trailing space, if any */
>  	for (; size && buf[size-1] == ' '; --size)
>  		buf[size-1] = '\0';
> -- 
> 2.25.1
>