lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <af05ea48-75c3-444c-86cb-ebf5c7bee2ec@mojatatu.com>
Date: Thu, 14 Mar 2024 11:47:15 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: renmingshuai <renmingshuai@...wei.com>, jiri@...nulli.us
Cc: caowangbao@...wei.com, davem@...emloft.net, jhs@...atatu.com,
 liaichun@...wei.com, netdev@...r.kernel.org, vladbu@...dia.com,
 xiyou.wangcong@...il.com, yanan@...wei.com
Subject: Re: [PATCH] net/sched: Forbid assigning mirred action to a filter
 attached to the egress

On 14/03/2024 11:04, renmingshuai wrote:
>>> ---
>>> net/sched/act_mirred.c                        |  4 +++
>>> .../tc-testing/tc-tests/actions/mirred.json   | 32 +++++++++++++++++++
>>> 2 files changed, 36 insertions(+)
>>>
>>> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
>>> index 5b3814365924..fc96705285fb 100644
>>> --- a/net/sched/act_mirred.c
>>> +++ b/net/sched/act_mirred.c
>>> @@ -120,6 +120,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
>>> 		NL_SET_ERR_MSG_MOD(extack, "Mirred requires attributes to be passed");
>>> 		return -EINVAL;
>>> 	}
>>> +	if (tp->chain->block->q->parent != TC_H_INGRESS) {
>>> +		NL_SET_ERR_MSG_MOD(extack, "Mirred can only be assigned to the filter attached to ingress");
>>
>> Hmm, that is quite restrictive. I'm pretty sure you would break some
>> valid usecases.
> Hmm, that is really quite restrictive. It might be better to Forbid mirred attached to egress filter
> to mirror or redirect packets to the egress.
> 
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index fc96705285fb..ab3841470992 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -152,6 +152,11 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
>                  return -EINVAL;
>          }
> 
> +       if ((tp->chain->block->q->parent != TC_H_INGRESS) && (parm->eaction == TCA_EGRESS_MIRROR || parm->eaction == TCA_EGRESS_REDIR)) {
> +               NL_SET_ERR_MSG_MOD(extack, "Mirred assigned to egress filter can only mirror or redirect to ingress");
> +               return -EINVAL;
> +       }
> +
>          switch (parm->eaction) {
>          case TCA_EGRESS_MIRROR:
>          case TCA_EGRESS_REDIR:


Are you sure this happens to egress mirred to netdevs other than the one 
used to attach the filter?
It seems like in your example you are forcing mirred egress to the same 
netdev as the filter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ