On Thu, Oct 26, 2017 at 11:26 AM, Davide Caratti <dcara...@redhat.com> wrote:
> the following script generates a NULL pointer dereference error:
>
> ip l a name eth0 type dummy
> tc q a dev eth0 parent :1 handle 1: htb
>
> upon creation of classful qdiscs, qdisc_alloc() dereferences dev_queue->dev
> assuming it is not NULL. This is not true when eth0 has been added, but not
> yet set administratively up; a bisect test proved that Linux started making
> NULL exception with the above two commands after commit 59cc1f61f09c ("net:
> sched:convert qdisc linked list to hashtable"). Let qdisc_alloc() return -1
> (-ENOENT) when a NULL value of dev_queue->dev is seen, so that non-crashing
> behaviour observable in Linux 4.8 is restored.
This analysis is wrong, you just hit noop_qdisc in this case.
Parent :1 doesn't exist in this case, so the second command
should be rejected since parent can't be found.
The the following patch works and better than your patch, but
still I think it should fail even earlier in this path, so there is probably
a even better fix.
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c6deb74e3d2f..6a3033c528a8 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1413,7 +1413,7 @@ static int tc_modify_qdisc(struct sk_buff *skb,
struct nlmsghdr *n,
if (p && p->ops->cl_ops && p->ops->cl_ops->select_queue)
dev_queue = p->ops->cl_ops->select_queue(p, tcm);
- else if (p)
+ else if (p && p != &noop_qdisc)
dev_queue = p->dev_queue;
else
dev_queue = netdev_get_tx_queue(dev, 0);
