Re: [nft PATCH] Kill the correct protocol expression during payload parsing

[Yanchuan Nian <ycnian@gmail.com>]

On Sat, Aug 30, 2014 at 11:52:49AM +0100, Patrick McHardy wrote:
> On Sat, Aug 30, 2014 at 01:17:15PM +0800, Yanchuan Nian wrote:
> > The protocol expression that should be killed when payload parsing
> > isn't the first one but the last one. Look at the result of this command:
> 
> That patch is competely wrong. Have you actually tested any other case?
> You're simply not killing any payload dependency anymore.
> 
> The correct fix is to check for OP_NEQ and deciding not to kill it based
> on that.
> 
Hi Patrick, Thanks to your replay. Yes, this patch is wrong. It was
careless of me forgetting to test it. I am sorry and I will try to fix
it. Thank you again.
> > 
> > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop
> > nft> list table ip filter
> > table ip filter {
> > 	chain input {
> > 		 type filter hook input priority 0;
> > 		 ip protocol tcp tcp sport http drop
> > 	}
> > }
> > nft>
> > 
> > With this patch, the result is:
> > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop
> > nft> list table ip filter
> > table ip filter {
> > 	chain input {
> > 		 type filter hook input priority 0;
> > 		 ip protocol != tcp tcp sport http drop
> > 	}
> > }
> > nft>
> > 
> > Signed-off-by: Yanchuan Nian <ycnian@gmail.com>
> > ---
> >  src/netlink_delinearize.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index 195d432..322c7cc 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -671,12 +671,11 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
> >  			nstmt = expr_stmt_alloc(&stmt->location, nexpr);
> >  			list_add_tail(&nstmt->list, &stmt->list);
> >  
> > -			/* Remember the first payload protocol expression to
> > +			/* Remember the last payload protocol expression to
> >  			 * kill it later on if made redundant by a higher layer
> >  			 * payload expression.
> >  			 */
> > -			if (ctx->pbase == PROTO_BASE_INVALID &&
> > -			    left->flags & EXPR_F_PROTOCOL)
> > +			if (left->flags & EXPR_F_PROTOCOL)
> >  				payload_dependency_store(ctx, nstmt,
> >  							 left->payload.base);
> >  			else
> > -- 
> > 1.9.3
> > 
>