diff mbox

[libnftnl] set: Fix nftnl_set_set_str

Message ID 20160627162425.4376-1-carlosfg@riseup.net
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Carlos Falgueras García June 27, 2016, 4:24 p.m. UTC
We need the string length

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carlos Falgueras García June 27, 2016, 4:29 p.m. UTC | #1
On 06/27/2016 06:24 PM, Carlos Falgueras García wrote:
> We need the string length
>
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/set.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/set.c b/src/set.c
> index 879100c..edbcbe5 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>
>  int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>  {
> -	return nftnl_set_set(s, attr, str);
> +	return nftnl_set_set_data(s, attr, str, strlen(str));
>  }
>  EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_attr_set_str);

This bug has gone unnoticed because all tests check if the set attribute 
was equal to the parsed one, but not check if it was really set. If you 
try to set an string but it fail, the test compares two empty strings 
and says that it is correct.

Maybe we can impove the tests if it checks too if the gotten attribute 
is equal to the set one.
--
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
Pablo Neira Ayuso July 1, 2016, 2:22 p.m. UTC | #2
On Mon, Jun 27, 2016 at 06:24:25PM +0200, Carlos Falgueras García wrote:
> We need the string length
> 
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/set.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/set.c b/src/set.c
> index 879100c..edbcbe5 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>  
>  int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>  {
> -	return nftnl_set_set(s, attr, str);
> +	return nftnl_set_set_data(s, attr, str, strlen(str));

I think this should be strlen(str) + 1 so we make sure the string is
nul-terminated.
--
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
Carlos Falgueras García July 1, 2016, 4:13 p.m. UTC | #3
On 01/07/16 16:22, Pablo Neira Ayuso wrote:
> On Mon, Jun 27, 2016 at 06:24:25PM +0200, Carlos Falgueras García wrote:
>> We need the string length
>>
>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>>   src/set.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/set.c b/src/set.c
>> index 879100c..edbcbe5 100644
>> --- a/src/set.c
>> +++ b/src/set.c
>> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>>   
>>   int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>>   {
>> -	return nftnl_set_set(s, attr, str);
>> +	return nftnl_set_set_data(s, attr, str, strlen(str));
> I think this should be strlen(str) + 1 so we make sure the string is
> nul-terminated.
Thanks Pablo. I have sent a v2 and another patch to fix this error in 
other code parts.
--
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/src/set.c b/src/set.c
index 879100c..edbcbe5 100644
--- a/src/set.c
+++ b/src/set.c
@@ -203,7 +203,7 @@  EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
 
 int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
 {
-	return nftnl_set_set(s, attr, str);
+	return nftnl_set_set_data(s, attr, str, strlen(str));
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_attr_set_str);