| Message ID | 20230703151038.157771-2-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/sched: sch_qfq: reintroduce lmax bound check for MTU | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | warning | CHECK: Unbalanced braces around else statement |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Mon, Jul 03, 2023 at 12:10:37PM -0300, Pedro Tammela wrote: > Commit 25369891fcef deletes a check for the case where no 'lmax' is > specified which commit 3037933448f6 fixes as 'lmax' > could be set to the device's MTU without any bound checking > for QFQ_LMAX_MIN and QFQ_LMAX_MAX. Therefore, reintroduce the check. > > Fixes: 25369891fcef ("net/sched: sch_qfq: refactor parsing of netlink parameters") > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > --- > net/sched/sch_qfq.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > index dfd9a99e6257..b624ae539c8c 100644 > --- a/net/sched/sch_qfq.c > +++ b/net/sched/sch_qfq.c > @@ -425,8 +425,15 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, > > if (tb[TCA_QFQ_LMAX]) > lmax = nla_get_u32(tb[TCA_QFQ_LMAX]); > - else > + else { > + /* MTU size is user controlled */ > lmax = psched_mtu(qdisc_dev(sch)); > + if (lmax < QFQ_MIN_LMAX || lmax > QFQ_MAX_LMAX) { > + NL_SET_ERR_MSG_MOD(extack, > + "MTU size out of bounds for qfq"); > + return -EINVAL; > + } > + } Hi Pedro, a minor nit from my side. If any arm of a condition has {}, then all should. if (...) { ... } else { ... } > > inv_w = ONE_FP / weight; > weight = ONE_FP / inv_w; > -- > 2.39.2 >
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c index dfd9a99e6257..b624ae539c8c 100644 --- a/net/sched/sch_qfq.c +++ b/net/sched/sch_qfq.c @@ -425,8 +425,15 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, if (tb[TCA_QFQ_LMAX]) lmax = nla_get_u32(tb[TCA_QFQ_LMAX]); - else + else { + /* MTU size is user controlled */ lmax = psched_mtu(qdisc_dev(sch)); + if (lmax < QFQ_MIN_LMAX || lmax > QFQ_MAX_LMAX) { + NL_SET_ERR_MSG_MOD(extack, + "MTU size out of bounds for qfq"); + return -EINVAL; + } + } inv_w = ONE_FP / weight; weight = ONE_FP / inv_w;
Commit 25369891fcef deletes a check for the case where no 'lmax' is specified which commit 3037933448f6 fixes as 'lmax' could be set to the device's MTU without any bound checking for QFQ_LMAX_MIN and QFQ_LMAX_MAX. Therefore, reintroduce the check. Fixes: 25369891fcef ("net/sched: sch_qfq: refactor parsing of netlink parameters") Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- net/sched/sch_qfq.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)