diff mbox series

net: sched: qfq: dont intepret cls results when asked to drop

Message ID 20230915142355.3411527-1-make_ruc2021@163.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: sched: qfq: dont intepret cls results when asked to drop | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1340 this patch: 1341
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang fail Errors and warnings before: 1363 this patch: 17
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 No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1363 this patch: 1364
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ma Ke Sept. 15, 2023, 2:23 p.m. UTC
If asked to drop a packet via TC_ACT_SHOT it is unsafe to
assume that res.class contains a valid pointer.

Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
 net/sched/sch_qfq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Vadim Fedorenko Sept. 15, 2023, 8:45 p.m. UTC | #1
On 15/09/2023 15:23, Ma Ke wrote:
> If asked to drop a packet via TC_ACT_SHOT it is unsafe to
> assume that res.class contains a valid pointer.
> 
> Signed-off-by: Ma Ke <make_ruc2021@163.com>
> ---
>   net/sched/sch_qfq.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 546c10adcacd..91c323eff012 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -696,6 +696,8 @@ static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
>   	fl = rcu_dereference_bh(q->filter_list);
>   	result = tcf_classify(skb, NULL, fl, &res, false);
>   	if (result >= 0) {
> +		if (result == TC_ACT_SHOT)
> +			return NULL;

The same comment again - the check is meaningless.

>   #ifdef CONFIG_NET_CLS_ACT
>   		switch (result) {
>   		case TC_ACT_QUEUED:
> @@ -703,8 +705,6 @@ static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
>   		case TC_ACT_TRAP:
>   			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
>   			fallthrough;
> -		case TC_ACT_SHOT:
> -			return NULL;
>   		}
>   #endif
>   		cl = (struct qfq_class *)res.class;
kernel test robot Sept. 16, 2023, 7:22 a.m. UTC | #2
Hi Ma,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]
[also build test ERROR on net/main linus/master v6.6-rc1 next-20230915]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ma-Ke/net-sched-qfq-dont-intepret-cls-results-when-asked-to-drop/20230915-222648
base:   net-next/main
patch link:    https://lore.kernel.org/r/20230915142355.3411527-1-make_ruc2021%40163.com
patch subject: [PATCH] net: sched: qfq: dont intepret cls results when asked to drop
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230916/202309161521.08Hs5BL9-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230916/202309161521.08Hs5BL9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309161521.08Hs5BL9-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/sched/sch_qfq.c:707:4: error: fallthrough annotation does not directly precede switch label
                           fallthrough;
                           ^
   include/linux/compiler_attributes.h:227:41: note: expanded from macro 'fallthrough'
   # define fallthrough                    __attribute__((__fallthrough__))
                                           ^
   1 error generated.


vim +707 net/sched/sch_qfq.c

0545a303777351 stephen hemminger   2011-04-04  678  
0545a303777351 stephen hemminger   2011-04-04  679  static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
0545a303777351 stephen hemminger   2011-04-04  680  				      int *qerr)
0545a303777351 stephen hemminger   2011-04-04  681  {
0545a303777351 stephen hemminger   2011-04-04  682  	struct qfq_sched *q = qdisc_priv(sch);
0545a303777351 stephen hemminger   2011-04-04  683  	struct qfq_class *cl;
0545a303777351 stephen hemminger   2011-04-04  684  	struct tcf_result res;
25d8c0d55f241c John Fastabend      2014-09-12  685  	struct tcf_proto *fl;
0545a303777351 stephen hemminger   2011-04-04  686  	int result;
0545a303777351 stephen hemminger   2011-04-04  687  
0545a303777351 stephen hemminger   2011-04-04  688  	if (TC_H_MAJ(skb->priority ^ sch->handle) == 0) {
0545a303777351 stephen hemminger   2011-04-04  689  		pr_debug("qfq_classify: found %d\n", skb->priority);
0545a303777351 stephen hemminger   2011-04-04  690  		cl = qfq_find_class(sch, skb->priority);
0545a303777351 stephen hemminger   2011-04-04  691  		if (cl != NULL)
0545a303777351 stephen hemminger   2011-04-04  692  			return cl;
0545a303777351 stephen hemminger   2011-04-04  693  	}
0545a303777351 stephen hemminger   2011-04-04  694  
0545a303777351 stephen hemminger   2011-04-04  695  	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
25d8c0d55f241c John Fastabend      2014-09-12  696  	fl = rcu_dereference_bh(q->filter_list);
3aa2605594556c Davide Caratti      2021-07-28  697  	result = tcf_classify(skb, NULL, fl, &res, false);
0545a303777351 stephen hemminger   2011-04-04  698  	if (result >= 0) {
0698b7004e7da6 Ma Ke               2023-09-15  699  		if (result == TC_ACT_SHOT)
0698b7004e7da6 Ma Ke               2023-09-15  700  			return NULL;
0545a303777351 stephen hemminger   2011-04-04  701  #ifdef CONFIG_NET_CLS_ACT
0545a303777351 stephen hemminger   2011-04-04  702  		switch (result) {
0545a303777351 stephen hemminger   2011-04-04  703  		case TC_ACT_QUEUED:
0545a303777351 stephen hemminger   2011-04-04  704  		case TC_ACT_STOLEN:
e25ea21ffa66a0 Jiri Pirko          2017-06-06  705  		case TC_ACT_TRAP:
0545a303777351 stephen hemminger   2011-04-04  706  			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
964201de695b8a Gustavo A. R. Silva 2020-07-07 @707  			fallthrough;
0545a303777351 stephen hemminger   2011-04-04  708  		}
0545a303777351 stephen hemminger   2011-04-04  709  #endif
0545a303777351 stephen hemminger   2011-04-04  710  		cl = (struct qfq_class *)res.class;
0545a303777351 stephen hemminger   2011-04-04  711  		if (cl == NULL)
0545a303777351 stephen hemminger   2011-04-04  712  			cl = qfq_find_class(sch, res.classid);
0545a303777351 stephen hemminger   2011-04-04  713  		return cl;
0545a303777351 stephen hemminger   2011-04-04  714  	}
0545a303777351 stephen hemminger   2011-04-04  715  
0545a303777351 stephen hemminger   2011-04-04  716  	return NULL;
0545a303777351 stephen hemminger   2011-04-04  717  }
0545a303777351 stephen hemminger   2011-04-04  718
diff mbox series

Patch

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 546c10adcacd..91c323eff012 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -696,6 +696,8 @@  static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 	fl = rcu_dereference_bh(q->filter_list);
 	result = tcf_classify(skb, NULL, fl, &res, false);
 	if (result >= 0) {
+		if (result == TC_ACT_SHOT)
+			return NULL;
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
 		case TC_ACT_QUEUED:
@@ -703,8 +705,6 @@  static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 		case TC_ACT_TRAP:
 			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
 			fallthrough;
-		case TC_ACT_SHOT:
-			return NULL;
 		}
 #endif
 		cl = (struct qfq_class *)res.class;