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 <fw@xxxxxxxxx> --- include/payload.h | 2 ++ src/netlink_delinearize.c | 31 +++++++++++++++++----- src/payload.c | 54 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/include/payload.h b/include/payload.h index 08e45f7f79e2..6685dad6f9f7 100644 --- a/include/payload.h +++ b/include/payload.h @@ -62,6 +62,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 046e7a472b8d..86c8602860f6 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2451,7 +2451,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, @@ -2501,17 +2501,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) @@ -3178,6 +3188,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) @@ -3211,13 +3222,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); @@ -3237,6 +3253,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); @@ -3248,7 +3265,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; @@ -3260,6 +3278,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 44aa834cc07b..f8b192b5f2fa 100644 --- a/src/payload.c +++ b/src/payload.c @@ -1046,6 +1046,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. @@ -1106,6 +1107,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 -- 2.45.3