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 <ffmanc...@riseup.net>

> CC: Florian Westphal <f...@strlen.de>
> CC: Pablo Neira Ayuso <pa...@netfilter.org>
> CC: netfilter-devel@vger.kernel.org
> Bug: https://bugzilla.netfilter.org/show_bug.cgi?id=1365
> Signed-off-by: Sergei Trofimovich <sly...@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");
>       }
>  }
>  
> 

Reply via email to