[Patch net] net_sched: reject TCF_EM_SIMPLE case for complex ematch module

[Patch net] net_sched: reject TCF_EM_SIMPLE case for complex ematch module

[Cong Wang]

From: Cong Wang <cong.wang@bytedance.com>

When TCF_EM_SIMPLE was introduced, it is supposed to be convenient
for ematch implementation:

https://lore.kernel.org/all/20050105110048.GO26856@postel.suug.ch/

"You don't have to, providing a 32bit data chunk without TCF_EM_SIMPLE
set will simply result in allocating & copy. It's an optimization,
nothing more."

So if an ematch module provides ops->datalen that means it wants a
complex data structure (saved in its em->data) instead of a simple u32
value. We should simply reject such a combination, otherwise this u32
could be misinterpreted as a pointer.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-and-tested-by: syzbot+4caeae4c7103813598ae@syzkaller.appspotmail.com
Reported-by: Jun Nie <jun.nie@linaro.org>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 net/sched/ematch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sched/ematch.c b/net/sched/ematch.c
index 4ce681361851..5c1235e6076a 100644
--- a/net/sched/ematch.c
+++ b/net/sched/ematch.c
@@ -255,6 +255,8 @@ static int tcf_em_validate(struct tcf_proto *tp,
 			 * the value carried.
 			 */
 			if (em_hdr->flags & TCF_EM_SIMPLE) {
+				if (em->ops->datalen > 0)
+					goto errout;
 				if (data_len < sizeof(u32))
 					goto errout;
 				em->data = *(u32 *) data;
-- 
2.34.1

Re: [Patch net] net_sched: reject TCF_EM_SIMPLE case for complex ematch module

[Paolo Abeni]

Hello,

On Sat, 2022-12-17 at 14:17 -0800, Cong Wang wrote:
> From: Cong Wang <cong.wang@bytedance.com>
> 
> When TCF_EM_SIMPLE was introduced, it is supposed to be convenient
> for ematch implementation:
> 
> https://lore.kernel.org/all/20050105110048.GO26856@postel.suug.ch/
> 
> "You don't have to, providing a 32bit data chunk without TCF_EM_SIMPLE
> set will simply result in allocating & copy. It's an optimization,
> nothing more."
> 
> So if an ematch module provides ops->datalen that means it wants a
> complex data structure (saved in its em->data) instead of a simple u32
> value. We should simply reject such a combination, otherwise this u32
> could be misinterpreted as a pointer.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-and-tested-by: syzbot+4caeae4c7103813598ae@syzkaller.appspotmail.com
> Reported-by: Jun Nie <jun.nie@linaro.org>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> ---
>  net/sched/ematch.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/sched/ematch.c b/net/sched/ematch.c
> index 4ce681361851..5c1235e6076a 100644
> --- a/net/sched/ematch.c
> +++ b/net/sched/ematch.c
> @@ -255,6 +255,8 @@ static int tcf_em_validate(struct tcf_proto *tp,
>  			 * the value carried.
>  			 */
>  			if (em_hdr->flags & TCF_EM_SIMPLE) {
> +				if (em->ops->datalen > 0)
> +					goto errout;
>  				if (data_len < sizeof(u32))
>  					goto errout;
>  				em->data = *(u32 *) data;


The patch LGTM, thanks!

Acked-by: Paolo Abeni <pabeni@redhat.com>

If I read correctly, this effectively rejects any ematch with
TCF_EM_SIMPLE set (all the existing tcf_ematch_ops structs have eiter
.change or . datalen > 0).

It looks like EM_SIMPLE does not work as intended since a lot of time
(possibly since its introduction !?!). Can we drop it completely?

Cheers,

Paolo

Re: [Patch net] net_sched: reject TCF_EM_SIMPLE case for complex ematch module

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Sat, 17 Dec 2022 14:17:07 -0800 you wrote:
> From: Cong Wang <cong.wang@bytedance.com>
> 
> When TCF_EM_SIMPLE was introduced, it is supposed to be convenient
> for ematch implementation:
> 
> https://lore.kernel.org/all/20050105110048.GO26856@postel.suug.ch/
> 
> [...]

Here is the summary with links:
  - [net] net_sched: reject TCF_EM_SIMPLE case for complex ematch module
    https://git.kernel.org/netdev/net/c/9cd3fd2054c3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html