netdev - Re: [Patch net-next 4/5] net_sched: forbid setting default qdisc to inappropriate ones

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:	Fri, 28 Aug 2015 14:39:45 -0700
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [Patch net-next 4/5] net_sched: forbid setting default qdisc to
 inappropriate ones

On Thu, Aug 27, 2015 at 9:23 PM, David Miller <davem@...emloft.net> wrote:
> From: Cong Wang <xiyou.wangcong@...il.com>
> Date: Thu, 27 Aug 2015 18:49:09 -0700
>
>> On Thu, Aug 27, 2015 at 4:18 PM, David Miller <davem@...emloft.net> wrote:
>>> From: Cong Wang <xiyou.wangcong@...il.com>
>>> Date: Thu, 27 Aug 2015 15:47:55 -0700
>>>
>>>> On Thu, Aug 27, 2015 at 3:42 PM, David Miller <davem@...emloft.net> wrote:
>>>>> If you fix it properly, by making every qdisc capable of being ->init()'d
>>>>> without explicit parameters, it will be the best behavior overall.
>>>>
>>>> The problem is ->init() is not even called when setting it as default,
>>>> since setting a default qdisc doesn't need to create a qdisc. This is
>>>> why the flag has to be in ops->flags rather than qdisc->flags.
>>>
>>> Just sounds like another shortcoming of how default qdiscs are handled,
>>
>> It has to, due to its definition. I don't see any other way except we change
>> the meaning of the default qdisc.
>
> We are talking past eachother.
>
> If a default qdisc like HTB is choosen, we invoke the ->init() function
> and we change the HTB ->init() function to do something reasonable
> if a NULL set of configuration attributes is given.  ie. make HTB use
> some defaults.
>
> Please explain to me why this won't fix the problem.

It does, and it is exactly what my local patch does.

The problem is setting HTB as default is already invalid from the
beginning, so HTB->init() is not supposed be called since we can
reject it earlier, and this is my whole point.

If HTB is not a good example, as using HTB as default might
make some sense, please try ingress qdisc, no error at _any_ time,
and apparently defaulting to ingress is totally non-sense.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html