sch_htb: Fix offload cleanup in htb_destroy on htb_init failure

htb_init may fail to do the offload if it's not supported or if a
runtime error happens when allocating direct qdiscs. In those cases
TC_HTB_CREATE command is not sent to the driver, however, htb_destroy
gets called anyway and attempts to send TC_HTB_DESTROY.

It shouldn't happen, because the driver didn't receive TC_HTB_CREATE,
and also because the driver may not support ndo_setup_tc at all, while
q->offload is true, and htb_destroy mistakenly thinks the offload is
supported. Trying to call ndo_setup_tc in the latter case will lead to a
NULL pointer dereference.

This commit fixes the issues with htb_destroy by deferring assignment of
q->offload until after the TC_HTB_CREATE command. The necessary cleanup
of the offload entities is already done in htb_init.

Reported-by: syzbot+b53a709f04722ca12a3c@syzkaller.appspotmail.com
Fixes: d03b195b5a ("sch_htb: Hierarchical QoS hardware offload")
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Maxim Mikityanskiy 2021-03-11 16:42:06 +02:00 коммит произвёл David S. Miller
Родитель 93bde210c4
Коммит fb3a3e37de
1 изменённых файлов: 9 добавлений и 6 удалений

Просмотреть файл

@ -1020,6 +1020,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
struct nlattr *tb[TCA_HTB_MAX + 1]; struct nlattr *tb[TCA_HTB_MAX + 1];
struct tc_htb_glob *gopt; struct tc_htb_glob *gopt;
unsigned int ntx; unsigned int ntx;
bool offload;
int err; int err;
qdisc_watchdog_init(&q->watchdog, sch); qdisc_watchdog_init(&q->watchdog, sch);
@ -1044,9 +1045,9 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
if (gopt->version != HTB_VER >> 16) if (gopt->version != HTB_VER >> 16)
return -EINVAL; return -EINVAL;
q->offload = nla_get_flag(tb[TCA_HTB_OFFLOAD]); offload = nla_get_flag(tb[TCA_HTB_OFFLOAD]);
if (q->offload) { if (offload) {
if (sch->parent != TC_H_ROOT) if (sch->parent != TC_H_ROOT)
return -EOPNOTSUPP; return -EOPNOTSUPP;
@ -1076,7 +1077,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
q->rate2quantum = 1; q->rate2quantum = 1;
q->defcls = gopt->defcls; q->defcls = gopt->defcls;
if (!q->offload) if (!offload)
return 0; return 0;
for (ntx = 0; ntx < q->num_direct_qdiscs; ntx++) { for (ntx = 0; ntx < q->num_direct_qdiscs; ntx++) {
@ -1107,12 +1108,14 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
if (err) if (err)
goto err_free_qdiscs; goto err_free_qdiscs;
/* Defer this assignment, so that htb_destroy skips offload-related
* parts (especially calling ndo_setup_tc) on errors.
*/
q->offload = true;
return 0; return 0;
err_free_qdiscs: err_free_qdiscs:
/* TC_HTB_CREATE call failed, avoid any further calls to the driver. */
q->offload = false;
for (ntx = 0; ntx < q->num_direct_qdiscs && q->direct_qdiscs[ntx]; for (ntx = 0; ntx < q->num_direct_qdiscs && q->direct_qdiscs[ntx];
ntx++) ntx++)
qdisc_put(q->direct_qdiscs[ntx]); qdisc_put(q->direct_qdiscs[ntx]);