| 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
| ||
|
Message-ID: <CAM0EoMkDhnm0QPtZEQPbnQtkfW7tTjHdv3fQoXzRXARVdhbc0A@mail.gmail.com>
Date: Thu, 4 Jan 2024 11:10:58 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, kuba@...nel.org, pabeni@...hat.com,
davem@...emloft.net, edumazet@...gle.com, xiyou.wangcong@...il.com,
victor@...atatu.com, pctammela@...atatu.com, idosch@...sch.org,
mleitner@...hat.com, vladbu@...dia.com, paulb@...dia.com
Subject: Re: [patch net-next] net: sched: move block device tracking into tcf_block_get/put_ext()
On Thu, Jan 4, 2024 at 7:58 AM Jiri Pirko <jiri@...nulli.us> wrote:
>
> From: Jiri Pirko <jiri@...dia.com>
>
> Inserting the device to block xarray in qdisc_create() is not suitable
> place to do this. As it requires use of tcf_block() callback, it causes
> multiple issues. It is called for all qdisc types, which is incorrect.
>
> So, instead, move it to more suitable place, which is tcf_block_get_ext()
> and make sure it is only done for qdiscs that use block infrastructure
> and also only for blocks which are shared.
>
> Symmetrically, alter the cleanup path, move the xarray entry removal
> into tcf_block_put_ext().
>
> Fixes: 913b47d3424e ("net/sched: Introduce tc block netdev tracking infra")
> Reported-by: Ido Schimmel <idosch@...dia.com>
> Closes: https://lore.kernel.org/all/ZY1hBb8GFwycfgvd@shredder/
> Reported-by: Kui-Feng Lee <sinquersw@...il.com>
> Closes: https://lore.kernel.org/all/ce8d3e55-b8bc-409c-ace9-5cf1c4f7c88e@gmail.com/
> Reported-and-tested-by: syzbot+84339b9e7330daae4d66@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/all/0000000000007c85f5060dcc3a28@google.com/
> Reported-and-tested-by: syzbot+806b0572c8d06b66b234@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/all/00000000000082f2f2060dcc3a92@google.com/
> Reported-and-tested-by: syzbot+0039110f932d438130f9@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/all/0000000000007fbc8c060dcc3a5c@google.com/
> Signed-off-by: Jiri Pirko <jiri@...dia.com>
Did you get a chance to run the tdc tests?
cheers,
jamal
> ---
> net/sched/cls_api.c | 14 ++++++++++++++
> net/sched/sch_api.c | 41 -----------------------------------------
> net/sched/sch_generic.c | 14 --------------
> 3 files changed, 14 insertions(+), 55 deletions(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index adf5de1ff773..253b26f2eddd 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1428,6 +1428,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
> struct tcf_block_ext_info *ei,
> struct netlink_ext_ack *extack)
> {
> + struct net_device *dev = qdisc_dev(q);
> struct net *net = qdisc_net(q);
> struct tcf_block *block = NULL;
> int err;
> @@ -1461,9 +1462,18 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
> if (err)
> goto err_block_offload_bind;
>
> + if (tcf_block_shared(block)) {
> + err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL);
> + if (err) {
> + NL_SET_ERR_MSG(extack, "block dev insert failed");
> + goto err_dev_insert;
> + }
> + }
> +
> *p_block = block;
> return 0;
>
> +err_dev_insert:
> err_block_offload_bind:
> tcf_chain0_head_change_cb_del(block, ei);
> err_chain0_head_change_cb_add:
> @@ -1502,8 +1512,12 @@ EXPORT_SYMBOL(tcf_block_get);
> void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
> struct tcf_block_ext_info *ei)
> {
> + struct net_device *dev = qdisc_dev(q);
> +
> if (!block)
> return;
> + if (tcf_block_shared(block))
> + xa_erase(&block->ports, dev->ifindex);
> tcf_chain0_head_change_cb_del(block, ei);
> tcf_block_owner_del(block, q, ei->binder_type);
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 2a2a48838eb9..36b025cc4fd2 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1209,43 +1209,6 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
> return 0;
> }
>
> -static int qdisc_block_add_dev(struct Qdisc *sch, struct net_device *dev,
> - struct netlink_ext_ack *extack)
> -{
> - const struct Qdisc_class_ops *cl_ops = sch->ops->cl_ops;
> - struct tcf_block *block;
> - int err;
> -
> - block = cl_ops->tcf_block(sch, TC_H_MIN_INGRESS, NULL);
> - if (block) {
> - err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL);
> - if (err) {
> - NL_SET_ERR_MSG(extack,
> - "ingress block dev insert failed");
> - return err;
> - }
> - }
> -
> - block = cl_ops->tcf_block(sch, TC_H_MIN_EGRESS, NULL);
> - if (block) {
> - err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL);
> - if (err) {
> - NL_SET_ERR_MSG(extack,
> - "Egress block dev insert failed");
> - goto err_out;
> - }
> - }
> -
> - return 0;
> -
> -err_out:
> - block = cl_ops->tcf_block(sch, TC_H_MIN_INGRESS, NULL);
> - if (block)
> - xa_erase(&block->ports, dev->ifindex);
> -
> - return err;
> -}
> -
> static int qdisc_block_indexes_set(struct Qdisc *sch, struct nlattr **tca,
> struct netlink_ext_ack *extack)
> {
> @@ -1416,10 +1379,6 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
> qdisc_hash_add(sch, false);
> trace_qdisc_create(ops, dev, parent);
>
> - err = qdisc_block_add_dev(sch, dev, extack);
> - if (err)
> - goto err_out4;
> -
> return sch;
>
> err_out4:
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index e33568df97a5..9b3e9262040b 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -1052,8 +1052,6 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
> {
> const struct Qdisc_ops *ops = qdisc->ops;
> struct net_device *dev = qdisc_dev(qdisc);
> - const struct Qdisc_class_ops *cops;
> - struct tcf_block *block;
>
> #ifdef CONFIG_NET_SCHED
> qdisc_hash_del(qdisc);
> @@ -1064,18 +1062,6 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
>
> qdisc_reset(qdisc);
>
> - cops = ops->cl_ops;
> - if (ops->ingress_block_get) {
> - block = cops->tcf_block(qdisc, TC_H_MIN_INGRESS, NULL);
> - if (block)
> - xa_erase(&block->ports, dev->ifindex);
> - }
> -
> - if (ops->egress_block_get) {
> - block = cops->tcf_block(qdisc, TC_H_MIN_EGRESS, NULL);
> - if (block)
> - xa_erase(&block->ports, dev->ifindex);
> - }
>
> if (ops->destroy)
> ops->destroy(qdisc);
> --
> 2.43.0
>
Powered by blists - more mailing lists