* [PATCH] nftables: don't crash in 'list ruleset' if policy is not set
@ 2019-09-16 7:33 Sergei Trofimovich
2019-09-16 7:41 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 3+ messages in thread
From: Sergei Trofimovich @ 2019-09-16 7:33 UTC (permalink / raw)
To: netfilter-devel; +Cc: Sergei Trofimovich, Florian Westphal, Pablo Neira Ayuso
Minimal reproducer:
```
$ cat nft.ruleset
# filters
table inet filter {
chain prerouting {
type filter hook prerouting priority -50
}
}
# dump new state
list ruleset
$ nft -c -f ./nft.ruleset
table inet filter {
chain prerouting {
Segmentation fault (core dumped)
```
The crash happens in `chain_print_declaration()`:
```
if (chain->flags & CHAIN_F_BASECHAIN) {
mpz_export_data(&policy, chain->policy->value,
BYTEORDER_HOST_ENDIAN, sizeof(int));
```
Here `chain->policy` is `NULL` (as textual rule does not mention it).
The change is not to print the policy if it's not set
(similar to `chain_evaluate()` handling).
CC: Florian Westphal <fw@strlen.de>
CC: Pablo Neira Ayuso <pablo@netfilter.org>
CC: netfilter-devel@vger.kernel.org
Bug: https://bugzilla.netfilter.org/show_bug.cgi?id=1365
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
src/rule.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index 5bb1c1d3..0cc1fa59 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1107,17 +1107,21 @@ static void chain_print_declaration(const struct chain *chain,
nft_print(octx, " # handle %" PRIu64, chain->handle.handle.id);
nft_print(octx, "\n");
if (chain->flags & CHAIN_F_BASECHAIN) {
- mpz_export_data(&policy, chain->policy->value,
- BYTEORDER_HOST_ENDIAN, sizeof(int));
nft_print(octx, "\t\ttype %s hook %s", chain->type,
hooknum2str(chain->handle.family, chain->hooknum));
if (chain->dev != NULL)
nft_print(octx, " device \"%s\"", chain->dev);
- nft_print(octx, " priority %s; policy %s;\n",
+ nft_print(octx, " priority %s;",
prio2str(octx, priobuf, sizeof(priobuf),
chain->handle.family, chain->hooknum,
- chain->priority.expr),
- chain_policy2str(policy));
+ chain->priority.expr));
+ if (chain->policy) {
+ mpz_export_data(&policy, chain->policy->value,
+ BYTEORDER_HOST_ENDIAN, sizeof(int));
+ nft_print(octx, " policy %s;",
+ chain_policy2str(policy));
+ }
+ nft_print(octx, "\n");
}
}
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nftables: don't crash in 'list ruleset' if policy is not set
2019-09-16 7:33 [PATCH] nftables: don't crash in 'list ruleset' if policy is not set Sergei Trofimovich
@ 2019-09-16 7:41 ` Fernando Fernandez Mancera
2019-09-16 7:52 ` Florian Westphal
0 siblings, 1 reply; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2019-09-16 7:41 UTC (permalink / raw)
To: Sergei Trofimovich, netfilter-devel; +Cc: Florian Westphal, Pablo Neira Ayuso
Hi Sergei,
On 9/16/19 9:33 AM, Sergei Trofimovich wrote:
> Minimal reproducer:
>
> ```
> $ cat nft.ruleset
> # filters
> table inet filter {
> chain prerouting {
> type filter hook prerouting priority -50
> }
> }
>
> # dump new state
> list ruleset
>
> $ nft -c -f ./nft.ruleset
> table inet filter {
> chain prerouting {
> Segmentation fault (core dumped)
> ```
>
> The crash happens in `chain_print_declaration()`:
>
> ```
> if (chain->flags & CHAIN_F_BASECHAIN) {
> mpz_export_data(&policy, chain->policy->value,
> BYTEORDER_HOST_ENDIAN, sizeof(int));
> ```
>
> Here `chain->policy` is `NULL` (as textual rule does not mention it).
>
> The change is not to print the policy if it's not set
> (similar to `chain_evaluate()` handling).
Thanks for fixing that. Sorry I missed that we could have a base chain
without policy.
Acked-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> CC: Florian Westphal <fw@strlen.de>
> CC: Pablo Neira Ayuso <pablo@netfilter.org>
> CC: netfilter-devel@vger.kernel.org
> Bug: https://bugzilla.netfilter.org/show_bug.cgi?id=1365
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
> src/rule.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/src/rule.c b/src/rule.c
> index 5bb1c1d3..0cc1fa59 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1107,17 +1107,21 @@ static void chain_print_declaration(const struct chain *chain,
> nft_print(octx, " # handle %" PRIu64, chain->handle.handle.id);
> nft_print(octx, "\n");
> if (chain->flags & CHAIN_F_BASECHAIN) {
> - mpz_export_data(&policy, chain->policy->value,
> - BYTEORDER_HOST_ENDIAN, sizeof(int));
> nft_print(octx, "\t\ttype %s hook %s", chain->type,
> hooknum2str(chain->handle.family, chain->hooknum));
> if (chain->dev != NULL)
> nft_print(octx, " device \"%s\"", chain->dev);
> - nft_print(octx, " priority %s; policy %s;\n",
> + nft_print(octx, " priority %s;",
> prio2str(octx, priobuf, sizeof(priobuf),
> chain->handle.family, chain->hooknum,
> - chain->priority.expr),
> - chain_policy2str(policy));
> + chain->priority.expr));
> + if (chain->policy) {
> + mpz_export_data(&policy, chain->policy->value,
> + BYTEORDER_HOST_ENDIAN, sizeof(int));
> + nft_print(octx, " policy %s;",
> + chain_policy2str(policy));
> + }
> + nft_print(octx, "\n");
> }
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] nftables: don't crash in 'list ruleset' if policy is not set
2019-09-16 7:41 ` Fernando Fernandez Mancera
@ 2019-09-16 7:52 ` Florian Westphal
0 siblings, 0 replies; 3+ messages in thread
From: Florian Westphal @ 2019-09-16 7:52 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: Sergei Trofimovich, netfilter-devel, Florian Westphal,
Pablo Neira Ayuso
Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:
> Hi Sergei,
>
> On 9/16/19 9:33 AM, Sergei Trofimovich wrote:
> > Minimal reproducer:
> >
> > ```
> > $ cat nft.ruleset
> > # filters
> > table inet filter {
> > chain prerouting {
> > type filter hook prerouting priority -50
> > }
> > }
> >
> > # dump new state
> > list ruleset
> >
> > $ nft -c -f ./nft.ruleset
> > table inet filter {
> > chain prerouting {
> > Segmentation fault (core dumped)
> > ```
> >
> > The crash happens in `chain_print_declaration()`:
> >
> > ```
> > if (chain->flags & CHAIN_F_BASECHAIN) {
> > mpz_export_data(&policy, chain->policy->value,
> > BYTEORDER_HOST_ENDIAN, sizeof(int));
> > ```
> >
> > Here `chain->policy` is `NULL` (as textual rule does not mention it).
> >
> > The change is not to print the policy if it's not set
> > (similar to `chain_evaluate()` handling).
>
> Thanks for fixing that. Sorry I missed that we could have a base chain
> without policy.
>
> Acked-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-16 7:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-16 7:33 [PATCH] nftables: don't crash in 'list ruleset' if policy is not set Sergei Trofimovich
2019-09-16 7:41 ` Fernando Fernandez Mancera
2019-09-16 7:52 ` Florian Westphal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.