| Message ID | 20240107144241.4169520-1-ap420073@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bec161add35b478a7746bf58bcdea6faa19129ef |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] amt: do not use overwrapped cb area | expand |
On Sun, 2024-01-07 at 14:42 +0000, Taehee Yoo wrote: > amt driver uses skb->cb for storing tunnel information. > This job is worked before TC layer and then amt driver load tunnel info > from skb->cb after TC layer. > So, its cb area should not be overwrapped with CB area used by TC. > In order to not use cb area used by TC, it skips the biggest cb > structure used by TC, which was qdisc_skb_cb. > But it's not anymore. > Currently, biggest structure of TC's CB is tc_skb_cb. > So, it should skip size of tc_skb_cb instead of qdisc_skb_cb. > > Fixes: ec624fe740b4 ("net/sched: Extend qdisc control block with tc control block") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> The patch LGTM, but I can't merge it ATM, as even the net tree is currently frozen until the net-next PR is merged back. Let me leave my tag explicitly to hopefully save some time for Jakub and/or Dave. Acked-by: Paolo Abeni <pabeni@redhat.com> Cheers, Paolo
On Sun, Jan 7, 2024 at 9:44 AM Taehee Yoo <ap420073@gmail.com> wrote: > > amt driver uses skb->cb for storing tunnel information. > This job is worked before TC layer and then amt driver load tunnel info > from skb->cb after TC layer. > So, its cb area should not be overwrapped with CB area used by TC. > In order to not use cb area used by TC, it skips the biggest cb > structure used by TC, which was qdisc_skb_cb. > But it's not anymore. > Currently, biggest structure of TC's CB is tc_skb_cb. > So, it should skip size of tc_skb_cb instead of qdisc_skb_cb. > > Fixes: ec624fe740b4 ("net/sched: Extend qdisc control block with tc control block") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal > --- > drivers/net/amt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/amt.c b/drivers/net/amt.c > index 53415e83821c..68e79b1272f6 100644 > --- a/drivers/net/amt.c > +++ b/drivers/net/amt.c > @@ -11,7 +11,7 @@ > #include <linux/net.h> > #include <linux/igmp.h> > #include <linux/workqueue.h> > -#include <net/sch_generic.h> > +#include <net/pkt_sched.h> > #include <net/net_namespace.h> > #include <net/ip.h> > #include <net/udp.h> > @@ -80,11 +80,11 @@ static struct mld2_grec mldv2_zero_grec; > > static struct amt_skb_cb *amt_skb_cb(struct sk_buff *skb) > { > - BUILD_BUG_ON(sizeof(struct amt_skb_cb) + sizeof(struct qdisc_skb_cb) > > + BUILD_BUG_ON(sizeof(struct amt_skb_cb) + sizeof(struct tc_skb_cb) > > sizeof_field(struct sk_buff, cb)); > > return (struct amt_skb_cb *)((void *)skb->cb + > - sizeof(struct qdisc_skb_cb)); > + sizeof(struct tc_skb_cb)); > } > > static void __amt_source_gc_work(void) > -- > 2.34.1 >
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sun, 7 Jan 2024 14:42:41 +0000 you wrote: > amt driver uses skb->cb for storing tunnel information. > This job is worked before TC layer and then amt driver load tunnel info > from skb->cb after TC layer. > So, its cb area should not be overwrapped with CB area used by TC. > In order to not use cb area used by TC, it skips the biggest cb > structure used by TC, which was qdisc_skb_cb. > But it's not anymore. > Currently, biggest structure of TC's CB is tc_skb_cb. > So, it should skip size of tc_skb_cb instead of qdisc_skb_cb. > > [...] Here is the summary with links: - [net] amt: do not use overwrapped cb area https://git.kernel.org/netdev/net/c/bec161add35b You are awesome, thank you!
diff --git a/drivers/net/amt.c b/drivers/net/amt.c index 53415e83821c..68e79b1272f6 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c @@ -11,7 +11,7 @@ #include <linux/net.h> #include <linux/igmp.h> #include <linux/workqueue.h> -#include <net/sch_generic.h> +#include <net/pkt_sched.h> #include <net/net_namespace.h> #include <net/ip.h> #include <net/udp.h> @@ -80,11 +80,11 @@ static struct mld2_grec mldv2_zero_grec; static struct amt_skb_cb *amt_skb_cb(struct sk_buff *skb) { - BUILD_BUG_ON(sizeof(struct amt_skb_cb) + sizeof(struct qdisc_skb_cb) > + BUILD_BUG_ON(sizeof(struct amt_skb_cb) + sizeof(struct tc_skb_cb) > sizeof_field(struct sk_buff, cb)); return (struct amt_skb_cb *)((void *)skb->cb + - sizeof(struct qdisc_skb_cb)); + sizeof(struct tc_skb_cb)); } static void __amt_source_gc_work(void)
amt driver uses skb->cb for storing tunnel information. This job is worked before TC layer and then amt driver load tunnel info from skb->cb after TC layer. So, its cb area should not be overwrapped with CB area used by TC. In order to not use cb area used by TC, it skips the biggest cb structure used by TC, which was qdisc_skb_cb. But it's not anymore. Currently, biggest structure of TC's CB is tc_skb_cb. So, it should skip size of tc_skb_cb instead of qdisc_skb_cb. Fixes: ec624fe740b4 ("net/sched: Extend qdisc control block with tc control block") Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- drivers/net/amt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)