netdev - Re: [PATCH net-next v8 1/5] net/sched: Introduce tc block netdev tracking infra

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]

Date: Thu, 28 Dec 2023 07:35:40 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: Victor Nogueira <victor@...atatu.com>, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, xiyou.wangcong@...il.com, 
	jiri@...nulli.us, mleitner@...hat.com, vladbu@...dia.com, paulb@...dia.com, 
	pctammela@...atatu.com, netdev@...r.kernel.org, kernel@...atatu.com
Subject: Re: [PATCH net-next v8 1/5] net/sched: Introduce tc block netdev
 tracking infra

On Thu, Dec 28, 2023 at 6:50 AM Ido Schimmel <idosch@...sch.org> wrote:
>
> On Tue, Dec 19, 2023 at 03:16:19PM -0300, Victor Nogueira wrote:
> > +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);
>
> Another problem, shouldn't there be a check that these operations are
> actually implemented? The following now crashes with a NULL pointer
> dereference:
>
> # tc qdisc replace dev swp1 root handle 1: tbf rate 1Mbit burst 256k limit 1M


I think this broke from v7->v8. Thanks for catching this. We'll send a
fix shortly.

cheers,
jamal

> > +     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)
> >  {
> > @@ -1350,6 +1387,10 @@ 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: