On Tue, Aug 13, 2019 at 08:44:06PM +0200, Florian Westphal wrote:
> table bla {
> chain foo { }
> chain bar { jump foo }
> }
> }
>
> Fails to restore on big-endian platforms:
> jump.nft:5:2-9: Error: Could not process rule: No such file or directory
> jump foo
>
> nft passes a 0-length name to the kernel.
>
> This is because when we export the value (the string), we provide
> the size of the destination buffer.
>
> In earlier versions, the parser allocated the name with the same
> fixed size and all was fine.
>
> After the fix, the export places the name in the wrong location
> in the destination buffer.
>
> This makes tests/shell/testcases/chains/0001jumps_0 work on s390x.
>
> Fixes: 142350f154c78 ("src: invalid read when importing chain name")
> Signed-off-by: Florian Westphal <[email protected]>
> ---
> src/datatype.c | 26 +++++++++++++++++---------
> src/netlink.c | 16 +++++++++++++---
> 2 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/src/datatype.c b/src/datatype.c
> index 28f726f4e84c..6908bc22d783 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -244,10 +244,24 @@ const struct datatype invalid_type = {
> .print = invalid_type_print,
> };
>
> -static void verdict_type_print(const struct expr *expr, struct output_ctx
> *octx)
> +static void verdict_jump_chain_print(const char *what, const struct expr *e,
> + struct output_ctx *octx)
> {
> char chain[NFT_CHAIN_MAXNAMELEN];
Probably:
chat chain[NFT_CHAIN_MAXNAMELEN + 1] = {};
to ensure space for \0.
> + unsigned int len;
> +
> + memset(chain, 0, sizeof(chain));
remove this memset then.
> + len = e->len / BITS_PER_BYTE;
div_round_up() ?
> + if (len >= sizeof(chain))
> + len = sizeof(chain) - 1;
Probably BUG() here instead if e->len > NFT_CHAIN_MAXNAMELEN? This
should not happen.
> +
> + mpz_export_data(chain, e->value, BYTEORDER_HOST_ENDIAN, len);
> + nft_print(octx, "%s %s", what, chain);
> +}
> +
> +static void verdict_type_print(const struct expr *expr, struct output_ctx
> *octx)
> +{
> switch (expr->verdict) {
> case NFT_CONTINUE:
> nft_print(octx, "continue");
> @@ -257,10 +271,7 @@ static void verdict_type_print(const struct expr *expr,
> struct output_ctx *octx)
> break;
> case NFT_JUMP:
> if (expr->chain->etype == EXPR_VALUE) {
> - mpz_export_data(chain, expr->chain->value,
> - BYTEORDER_HOST_ENDIAN,
> - NFT_CHAIN_MAXNAMELEN);
> - nft_print(octx, "jump %s", chain);
> + verdict_jump_chain_print("jump", expr->chain, octx);
> } else {
> nft_print(octx, "jump ");
> expr_print(expr->chain, octx);
> @@ -268,10 +279,7 @@ static void verdict_type_print(const struct expr *expr,
> struct output_ctx *octx)
> break;
> case NFT_GOTO:
> if (expr->chain->etype == EXPR_VALUE) {
> - mpz_export_data(chain, expr->chain->value,
> - BYTEORDER_HOST_ENDIAN,
> - NFT_CHAIN_MAXNAMELEN);
> - nft_print(octx, "goto %s", chain);
> + verdict_jump_chain_print("goto", expr->chain, octx);
> } else {
> nft_print(octx, "goto ");
> expr_print(expr->chain, octx);
> diff --git a/src/netlink.c b/src/netlink.c
> index aeeb12eaca93..f8e1120447d9 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -222,17 +222,27 @@ static void netlink_gen_verdict(const struct expr *expr,
> struct nft_data_linearize *data)
> {
> char chain[NFT_CHAIN_MAXNAMELEN];
...[NFT_CHAIN_MAXNAMELEN + 1] = {};
> + unsigned int len;
>
> data->verdict = expr->verdict;
>
> switch (expr->verdict) {
> case NFT_JUMP:
> case NFT_GOTO:
> + len = expr->chain->len / BITS_PER_BYTE;
div_round_up()
> +
> + if (!len)
> + BUG("chain length is 0");
> +
> + if (len > sizeof(chain))
> + BUG("chain is too large (%u, %u max)",
> + len, (unsigned int)sizeof(chain));
> +
> + memset(chain, 0, sizeof(chain));
> +
> mpz_export_data(chain, expr->chain->value,
> - BYTEORDER_HOST_ENDIAN,
> - NFT_CHAIN_MAXNAMELEN);
> + BYTEORDER_HOST_ENDIAN, len);
> snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain);
> - data->chain[NFT_CHAIN_MAXNAMELEN-1] = '\0';
> break;
> }
> }
> --
> 2.21.0
>
