From: Dan Carpenter <dan.carpenter@oracle.com>
To: kaber@trash.net
Cc: netfilter-devel@vger.kernel.org
Subject: [bug report] ct_sip_parse_numerical_param() error handling
Date: Thu, 26 Feb 2015 17:22:04 +0300 [thread overview]
Message-ID: <20150226142204.GA14815@mwanda> (raw)
Hello Patrick McHardy,
The patch 2bbb21168a90: "[NETFILTER]: nf_conntrack_sip: introduce URI
and header parameter parsing helpers" from Mar 25, 2008, leads to the
following static checker warning:
net/netfilter/nf_conntrack_sip.c:1230 process_register_request()
warn: bool is not less than zero.
net/netfilter/nf_conntrack_sip.c:1336 process_register_response()
warn: bool is not less than zero.
The problem is ct_sip_parse_numerical_param() returns zero on failure
but two of the callers expect negative error codes.
net/netfilter/nf_conntrack_sip.c
1307 if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,
1308 &matchoff, &matchlen) > 0)
1309 expires = simple_strtoul(*dptr + matchoff, NULL, 10);
^^^^^^^
We set expires.
1310
1311 while (1) {
1312 unsigned int c_expires = expires;
^^^^^^^^^^^^^^^^^^^^
and c_expires.
1313
1314 ret = ct_sip_parse_header_uri(ct, *dptr, &coff, *datalen,
1315 SIP_HDR_CONTACT, &in_contact,
1316 &matchoff, &matchlen,
1317 &addr, &port);
1318 if (ret < 0) {
1319 nf_ct_helper_log(skb, ct, "cannot parse contact");
1320 return NF_DROP;
1321 } else if (ret == 0)
1322 break;
1323
1324 /* We don't support third-party registrations */
1325 if (!nf_inet_addr_cmp(&ct->tuplehash[dir].tuple.dst.u3, &addr))
1326 continue;
1327
1328 if (ct_sip_parse_transport(ct, *dptr, matchoff + matchlen,
1329 *datalen, &proto) == 0)
1330 continue;
1331
1332 ret = ct_sip_parse_numerical_param(ct, *dptr,
1333 matchoff + matchlen,
1334 *datalen, "expires=",
1335 NULL, NULL, &c_expires);
^^^^^^^^^
1336 if (ret < 0) {
^^^^^^^
"ret" is never negative.
1337 nf_ct_helper_log(skb, ct, "cannot parse expires");
1338 return NF_DROP;
1339 }
1340 if (c_expires == 0)
^^^^^^^^^^^^^^
On the first error path in ct_sip_parse_numerical_param(), if
ct_sip_header_search() failes then we return the original c_expires. On
the other error path we set it to zero.
1341 break;
1342 if (refresh_signalling_expectation(ct, &addr, proto, port,
1343 c_expires))
1344 return NF_ACCEPT;
1345 }
1346
regards,
dan carpenter
reply other threads:[~2015-02-26 14:22 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150226142204.GA14815@mwanda \
--to=dan.carpenter@oracle.com \
--cc=kaber@trash.net \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).