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
| ||
|
Date: Thu, 30 Jul 2020 14:50:19 -0700 From: Cong Wang <xiyou.wangcong@...il.com> To: wenxu <wenxu@...oud.cn> Cc: Linux Kernel Network Developers <netdev@...r.kernel.org> Subject: Re: [PATCH net] net/sched: act_ct: fix miss set mru for ovs after defrag in act_ct On Thu, Jul 30, 2020 at 1:53 AM wenxu <wenxu@...oud.cn> wrote: > > > On 7/30/2020 2:03 PM, Cong Wang wrote: > > On Wed, Jul 29, 2020 at 3:41 AM <wenxu@...oud.cn> wrote: > >> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > >> index c510b03..45401d5 100644 > >> --- a/include/net/sch_generic.h > >> +++ b/include/net/sch_generic.h > >> @@ -384,6 +384,7 @@ struct qdisc_skb_cb { > >> }; > >> #define QDISC_CB_PRIV_LEN 20 > >> unsigned char data[QDISC_CB_PRIV_LEN]; > >> + u16 mru; > >> }; > > Hmm, can you put it in the anonymous struct before 'data'? > > > > We validate this cb size and data size like blow: > > > > static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz) > > { > > struct qdisc_skb_cb *qcb; > > > > BUILD_BUG_ON(sizeof(skb->cb) < offsetof(struct qdisc_skb_cb, > > data) + sz); > > BUILD_BUG_ON(sizeof(qcb->data) < sz); > > } > > > > It _kinda_ expects ->data at the end. > > It seems the data offsetof data should be align with szieof(u64)? > > If I modify it as following > > @@ -383,6 +383,9 @@ struct qdisc_skb_cb { > unsigned int pkt_len; > u16 slave_dev_queue_mapping; > u16 tc_classid; > + u16 mru; > }; > #define QDISC_CB_PRIV_LEN 20 > unsigned char data[QDISC_CB_PRIV_LEN]; > > compile fail: > > net/core/filter.c:7625:3: note: in expansion of macro ‘BUILD_BUG_ON’ > BUILD_BUG_ON((offsetof(struct sk_buff, cb) + > > inn the file: net/core/filter.c > > case offsetof(struct __sk_buff, cb[0]) ... > > offsetofend(struct __sk_buff, cb[4]) - 1: > BUILD_BUG_ON(sizeof_field(struct qdisc_skb_cb, data) < 20); > BUILD_BUG_ON((offsetof(struct sk_buff, cb) + > offsetof(struct qdisc_skb_cb, data)) % > sizeof(__u64)); > > > If I modify it as following > > @@ -383,6 +383,9 @@ struct qdisc_skb_cb { > unsigned int pkt_len; > u16 slave_dev_queue_mapping; > u16 tc_classid; > + u16 mru; > + u16 _pad1; > + u32 _pad2; > }; > #define QDISC_CB_PRIV_LEN 20 > unsigned char data[QDISC_CB_PRIV_LEN]; > > > compile fail: > > ./include/linux/filter.h:633:2: note: in expansion of macro ‘BUILD_BUG_ON’ > BUILD_BUG_ON(sizeof(*cb) > sizeof_field(struct sk_buff, cb)); > > > static inline void bpf_compute_data_pointers(struct sk_buff *skb) > { > struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb; > > BUILD_BUG_ON(sizeof(*cb) > sizeof_field(struct sk_buff, cb)); > cb->data_meta = skb->data - skb_metadata_len(skb); > cb->data_end = skb->data + skb_headlen(skb); > } > > > It seems no space for new value add before 'data'? Hmm, I didn't know bpf has such restrictions on qdisc_skb_cb. It seems impossible to add a new field before data, if you keep it after data, can you adjust qdisc_cb_private_validate() too, like below? diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index c510b03b9751..68235489a5d4 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -463,7 +463,7 @@ static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz) { struct qdisc_skb_cb *qcb; - BUILD_BUG_ON(sizeof(skb->cb) < offsetof(struct qdisc_skb_cb, data) + sz); + BUILD_BUG_ON(sizeof(skb->cb) < sizeof(*qcb)); BUILD_BUG_ON(sizeof(qcb->data) < sz); } Thanks.
Powered by blists - more mailing lists