From: Florian Westphal Date: Thu, 30 Jan 2025 17:47:13 +0000 (+0100) Subject: src: add and use payload_expr_trim_force X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=99a02112a3372a674c3fc7ce779f251b9e616115;p=thirdparty%2Fnftables.git src: add and use payload_expr_trim_force commit 4f046ae450cbe2567022575c11dd65a9d9ea272d upstream. Previous commit fixed erroneous handling of raw expressions when RHS sets a zero value. Input: @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0 Output:@ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set \ @ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000 After this patch, this will instead display: @ih,58,6 set 0x0 @ih,86,6 set 0x0 @ih,170,22 set 0x0 payload_expr_trim_force() only works when the payload has no known protocol (template) attached, i.e. will be printed as raw payload syntax. It performs sanity checks on @mask and then adjusts the payload expression length and offset according to the mask. Also add this check in __binop_postprocess() so we can also discard masks when matching, e.g. '@ih,7,5 2' becomes '@ih,7,5 0x2', not '@ih,0,16 & 0xffc0 == 0x20'. binop_postprocess now returns if it performed an action or not; if this returns true then arguments might have been freed so callers must no longer refer to any of the expressions attached to the binop. Next patch adds test cases for this. Signed-off-by: Florian Westphal Reviewed-by: Pablo Neira Ayuso --- diff --git a/include/payload.h b/include/payload.h index 37869928..d207aabb 100644 --- a/include/payload.h +++ b/include/payload.h @@ -60,6 +60,8 @@ extern struct expr *payload_expr_join(const struct expr *e1, bool payload_expr_trim(struct expr *expr, struct expr *mask, const struct proto_ctx *ctx, unsigned int *shift); +bool payload_expr_trim_force(struct expr *expr, struct expr *mask, + unsigned int *shift); extern void payload_expr_expand(struct list_head *list, struct expr *expr, const struct proto_ctx *ctx); extern void payload_expr_complete(struct expr *expr, diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index b04cc0b0..4b4898b0 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2270,7 +2270,7 @@ static void binop_adjust(const struct expr *binop, struct expr *right, } } -static void __binop_postprocess(struct rule_pp_ctx *ctx, +static bool __binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr, struct expr *left, struct expr *mask, @@ -2319,17 +2319,27 @@ static void __binop_postprocess(struct rule_pp_ctx *ctx, expr_set_type(right, left->dtype, left->byteorder); expr_free(binop); + return true; + } else if (left->etype == EXPR_PAYLOAD && + expr->right->etype == EXPR_VALUE && + payload_expr_trim_force(left, mask, &shift)) { + mpz_rshift_ui(expr->right->value, shift); + *expr_binop = expr_get(left); + expr_free(binop); + return true; } + + return false; } -static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr, +static bool binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr, struct expr **expr_binop) { struct expr *binop = *expr_binop; struct expr *left = binop->left; struct expr *mask = binop->right; - __binop_postprocess(ctx, expr, left, mask, expr_binop); + return __binop_postprocess(ctx, expr, left, mask, expr_binop); } static void map_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr) @@ -2984,6 +2994,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx) switch (expr->left->etype) { case EXPR_BINOP: {/* I? */ + unsigned int shift = 0; mpz_t tmp; if (expr->op != OP_OR) @@ -3017,13 +3028,18 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx) mpz_set(mask->value, bitmask); mpz_clear(bitmask); - binop_postprocess(ctx, expr, &expr->left); - if (!payload_is_known(payload)) { + if (!binop_postprocess(ctx, expr, &expr->left) && + !payload_is_known(payload) && + !payload_expr_trim_force(payload, + mask, &shift)) { mpz_set(mask->value, tmp); mpz_clear(tmp); return; } + if (shift) + mpz_rshift_ui(value->value, shift); + mpz_clear(tmp); expr_free(stmt->payload.expr); stmt->payload.expr = expr_get(payload); @@ -3043,6 +3059,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx) switch (expr->op) { case OP_AND: { /* IIa */ + unsigned int shift_unused; mpz_t tmp; mpz_init(tmp); @@ -3054,7 +3071,8 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx) mpz_clear(bitmask); stmt_payload_binop_pp(ctx, expr); - if (!payload_is_known(expr->left)) { + if (!payload_is_known(expr->left) && + !payload_expr_trim_force(expr->left, mask, &shift_unused)) { mpz_set(mask->value, tmp); mpz_clear(tmp); return; @@ -3066,6 +3084,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx) * clear the payload expression. * The "mask" value becomes new stmt->payload.value * so set this to 0. + * Also the reason why &shift_unused is ignored. */ mpz_set_ui(mask->value, 0); break; diff --git a/src/payload.c b/src/payload.c index c8428885..60c2cf2c 100644 --- a/src/payload.c +++ b/src/payload.c @@ -973,6 +973,7 @@ static unsigned int mask_length(const struct expr *mask) * @expr: the payload expression * @mask: mask to use when searching templates * @ctx: protocol context + * @shift: shift adjustment to fix up RHS value * * Walk the template list and determine if a match can be found without * using the provided mask. @@ -1033,6 +1034,59 @@ bool payload_expr_trim(struct expr *expr, struct expr *mask, return false; } +/** + * payload_expr_trim_force - adjust payload len/offset according to mask + * + * @expr: the payload expression + * @mask: mask to use when searching templates + * @shift: shift adjustment to fix up RHS value + * + * Force-trim an unknown payload expression according to mask. + * + * This is only useful for unkown payload expressions that need + * to be printed in raw syntax (@base,offset,length). The kernel + * can only deal with byte-divisible offsets/length, e.g. @th,16,8. + * In such case we might be able to get rid of the mask. + * @base,offset,length & MASK OPERATOR VALUE then becomes + * @base,offset,length VALUE, where at least one of offset increases + * and length decreases. + * + * This function also returns the shift for the right hand + * constant side of the expression. + * + * @return: true if @expr was adjusted and mask can be discarded. + */ +bool payload_expr_trim_force(struct expr *expr, struct expr *mask, unsigned int *shift) +{ + unsigned int payload_offset = expr->payload.offset; + unsigned int mask_len = mask_length(mask); + unsigned int off, real_len; + + if (payload_is_known(expr) || expr->len <= mask_len) + return false; + + /* This provides the payload offset to use. + * mask->len is the total length of the mask, e.g. 16. + * mask_len holds the last bit number that will be zeroed, + */ + off = round_up(mask->len, BITS_PER_BYTE) - mask_len; + payload_offset += off; + + /* kernel only allows offsets <= 255 */ + if (round_up(payload_offset, BITS_PER_BYTE) > 255) + return false; + + real_len = mpz_popcount(mask->value); + if (real_len > expr->len) + return false; + + expr->payload.offset = payload_offset; + expr->len = real_len; + + *shift = mask_to_offset(mask); + return true; +} + /** * payload_expr_expand - expand raw merged adjacent payload expressions into its * original components