2023-06-23 11:45:48

by Gavrilov Ilia

[permalink] [raw]
Subject: [PATCH net] netfilter: nf_conntrack_sip: fix the ct_sip_parse_numerical_param() return value.

From: "Ilia.Gavrilov" <[email protected]>

ct_sip_parse_numerical_param() returns only 0 or 1 now.
But process_register_request() and process_register_response() imply
checking for a negative value if parsing of a numerical header parameter
failed.
The invocation in nf_nat_sip() looks correct:
if (ct_sip_parse_numerical_param(...) > 0 &&
...) { ... }

Make the return value of the function ct_sip_parse_numerical_param()
a tristate to fix all the cases
a) return 1 if value is found; *val is set
b) return 0 if value is not found; *val is unchanged
c) return -1 on error; *val is undefined

Found by InfoTeCS on behalf of Linux Verification Center
(linuxtesting.org) with SVACE.

Fixes: 0f32a40fc91a ("[NETFILTER]: nf_conntrack_sip: create signalling expectations")
Reviewed-by: Simon Horman <[email protected]>
Signed-off-by: Ilia.Gavrilov <[email protected]>
---
- Fix description
- Repost according
https://lore.kernel.org/all/[email protected]/
net/netfilter/nf_conntrack_sip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 77f5e82d8e3f..d0eac27f6ba0 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -611,7 +611,7 @@ int ct_sip_parse_numerical_param(const struct nf_conn *ct, const char *dptr,
start += strlen(name);
*val = simple_strtoul(start, &end, 0);
if (start == end)
- return 0;
+ return -1;
if (matchoff && matchlen) {
*matchoff = start - dptr;
*matchlen = end - start;
--
2.39.2


2023-06-26 16:11:47

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH net] netfilter: nf_conntrack_sip: fix the ct_sip_parse_numerical_param() return value.

On Fri, Jun 23, 2023 at 11:23:46AM +0000, Gavrilov Ilia wrote:
> From: "Ilia.Gavrilov" <[email protected]>
>
> ct_sip_parse_numerical_param() returns only 0 or 1 now.
> But process_register_request() and process_register_response() imply
> checking for a negative value if parsing of a numerical header parameter
> failed.
> The invocation in nf_nat_sip() looks correct:
> if (ct_sip_parse_numerical_param(...) > 0 &&
> ...) { ... }
>
> Make the return value of the function ct_sip_parse_numerical_param()
> a tristate to fix all the cases
> a) return 1 if value is found; *val is set
> b) return 0 if value is not found; *val is unchanged
> c) return -1 on error; *val is undefined

Applied to nf.git