[PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Du Cheng]

There is a reproducible sequence from the userland that will trigger a WARN_ON()
condition in taprio_get_start_time, which causes kernel to panic if configured
as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
userland-initiated syscalls.

Reported as bug on syzkaller:
https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c

Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
Signed-off-by: Du Cheng <ducheng2@gmail.com>
---
Detailed explanation:

In net/sched/sched_taprio.c:999
The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).

sched->cycle_time is accumulated within `parse_taprio_schedule()` during
`taprio_init()`, in the following 2 ways:

1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);

note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:

If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).

Reliable reproducable steps:
1. add net device team0 
2. add team_slave_0, team_slave_1
3. sendmsg(struct msghdr {
	.iov = struct nlmsghdr {
		.type = RTM_NEWQDISC,
	}
	struct tcmsg {
		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
		.nlattr[] = {
			TCA_KIND: "taprio",
			TCA_OPTIONS: {
				.nlattr = {
					TCA_TAPRIO_ATTR_PRIOMAP: ...,
					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
				}
			}
		}
	}
}

Callstack:

parse_taprio_schedule()
taprio_change()
taprio_init()
qdisc_create()
tc_modify_qdisc()
rtnetlink_rcv_msg()
...
sendmsg()

These steps are extracted from syzkaller reproducer:
https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000

 net/sched/sch_taprio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 8287894541e3..5f2ff0f15d5c 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
 	 * something went really wrong. In that case, we should warn about this
 	 * inconsistent state and return error.
 	 */
-	if (WARN_ON(!cycle))
+	if (!cycle) {
 		return -EFAULT;
 
 	/* Schedule the start time for the beginning of the next
-- 
2.30.2

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Eric Dumazet]

On 4/15/21 8:39 AM, Du Cheng wrote:
> There is a reproducible sequence from the userland that will trigger a WARN_ON()
> condition in taprio_get_start_time, which causes kernel to panic if configured
> as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
> userland-initiated syscalls.
> 
> Reported as bug on syzkaller:
> https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c
> 
> Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
> Signed-off-by: Du Cheng <ducheng2@gmail.com>
> ---
> Detailed explanation:
> 
> In net/sched/sched_taprio.c:999
> The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
> comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).
> 
> sched->cycle_time is accumulated within `parse_taprio_schedule()` during
> `taprio_init()`, in the following 2 ways:
> 
> 1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
> 2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);
> 
> note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:
> 
> If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
> TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
> in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).
> 
> Reliable reproducable steps:
> 1. add net device team0 
> 2. add team_slave_0, team_slave_1
> 3. sendmsg(struct msghdr {
> 	.iov = struct nlmsghdr {
> 		.type = RTM_NEWQDISC,
> 	}
> 	struct tcmsg {
> 		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
> 		.nlattr[] = {
> 			TCA_KIND: "taprio",
> 			TCA_OPTIONS: {
> 				.nlattr = {
> 					TCA_TAPRIO_ATTR_PRIOMAP: ...,
> 					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
> 					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
> 				}
> 			}
> 		}
> 	}
> }
> 
> Callstack:
> 
> parse_taprio_schedule()
> taprio_change()
> taprio_init()
> qdisc_create()
> tc_modify_qdisc()
> rtnetlink_rcv_msg()
> ...
> sendmsg()
> 
> These steps are extracted from syzkaller reproducer:
> https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000
> 
>  net/sched/sch_taprio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 8287894541e3..5f2ff0f15d5c 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
>  	 * something went really wrong. In that case, we should warn about this
>  	 * inconsistent state and return error.
>  	 */
> -	if (WARN_ON(!cycle))
> +	if (!cycle) {
>  		return -EFAULT;
>  
>  	/* Schedule the start time for the beginning of the next
> 

This 'fix' is wrong, not even compiled, thus not tested.

 sched->cycle_time MUST not be zero ever, there are plenty
of other places where other crashes will happen.

You are silencing a condition that should be caught much earlier.

There is even a fat comment to explain this, that can be partially seen in your patch.


Correct fix will probably be :

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 8287894541e3ce5f290be2e592c0dcbdf2ec6b60..189c617a582a2eecc92e35187379d4c2889289df 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -901,6 +901,8 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb,
 
                list_for_each_entry(entry, &new->entries, list)
                        cycle = ktime_add_ns(cycle, entry->interval);
+               if (!cycle)
+                       return -EINVAL;
                new->cycle_time = cycle;
        }
 

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Du Cheng]

Le Thu, Apr 15, 2021 at 08:56:09AM +0200, Eric Dumazet a écrit :
> 
> 
> On 4/15/21 8:39 AM, Du Cheng wrote:
> > There is a reproducible sequence from the userland that will trigger a WARN_ON()
> > condition in taprio_get_start_time, which causes kernel to panic if configured
> > as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
> > userland-initiated syscalls.
> > 
> > Reported as bug on syzkaller:
> > https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c
> > 
> > Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
> > Signed-off-by: Du Cheng <ducheng2@gmail.com>
> > ---
> > Detailed explanation:
> > 
> > In net/sched/sched_taprio.c:999
> > The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
> > comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).
> > 
> > sched->cycle_time is accumulated within `parse_taprio_schedule()` during
> > `taprio_init()`, in the following 2 ways:
> > 
> > 1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
> > 2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);
> > 
> > note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:
> > 
> > If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
> > TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
> > in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).
> > 
> > Reliable reproducable steps:
> > 1. add net device team0 
> > 2. add team_slave_0, team_slave_1
> > 3. sendmsg(struct msghdr {
> > 	.iov = struct nlmsghdr {
> > 		.type = RTM_NEWQDISC,
> > 	}
> > 	struct tcmsg {
> > 		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
> > 		.nlattr[] = {
> > 			TCA_KIND: "taprio",
> > 			TCA_OPTIONS: {
> > 				.nlattr = {
> > 					TCA_TAPRIO_ATTR_PRIOMAP: ...,
> > 					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
> > 					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
> > 				}
> > 			}
> > 		}
> > 	}
> > }
> > 
> > Callstack:
> > 
> > parse_taprio_schedule()
> > taprio_change()
> > taprio_init()
> > qdisc_create()
> > tc_modify_qdisc()
> > rtnetlink_rcv_msg()
> > ...
> > sendmsg()
> > 
> > These steps are extracted from syzkaller reproducer:
> > https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000
> > 
> >  net/sched/sch_taprio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > index 8287894541e3..5f2ff0f15d5c 100644
> > --- a/net/sched/sch_taprio.c
> > +++ b/net/sched/sch_taprio.c
> > @@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
> >  	 * something went really wrong. In that case, we should warn about this
> >  	 * inconsistent state and return error.
> >  	 */
> > -	if (WARN_ON(!cycle))
> > +	if (!cycle) {
> >  		return -EFAULT;
> >  
> >  	/* Schedule the start time for the beginning of the next
> > 
> 
> This 'fix' is wrong, not even compiled, thus not tested.
> 
>  sched->cycle_time MUST not be zero ever, there are plenty
> of other places where other crashes will happen.
> 
> You are silencing a condition that should be caught much earlier.
> 
> There is even a fat comment to explain this, that can be partially seen in your patch.
> 
> 
> Correct fix will probably be :
> 
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 8287894541e3ce5f290be2e592c0dcbdf2ec6b60..189c617a582a2eecc92e35187379d4c2889289df 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -901,6 +901,8 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb,
>  
>                 list_for_each_entry(entry, &new->entries, list)
>                         cycle = ktime_add_ns(cycle, entry->interval);
> +               if (!cycle)
> +                       return -EINVAL;
>                 new->cycle_time = cycle;
>         }
>  
> 
> 

Hi Eric,

Sorry for the typo in the previous PATCH, I must have let the '{' slip out
, when I tried to cleanup my own debugging lines.

Anyway, my intention was indeed to silently return error back to the userland,
if the cycle is 0. The removal of WARN_ON() is so that this would not cause the
kernel to panic.

`cycle` can be 0 if the userland provides invalid input, which should be
anticipated and gracefully handled, hence panic would be too dramatic.

With the removal of WARN_ON(), the original logic was not
altered: return -EINVAL to the userland and abort subsequent steps.

I will later submit a v2 to correct my typo.
Thank you for your input.

Regards,
Du Cheng

[PATCH v2] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Du Cheng]

There is a reproducible sequence from the userland that will trigger a WARN_ON()
condition in taprio_get_start_time, which causes kernel to panic if configured
as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
userland-initiated syscalls.

Reported as bug on syzkaller:
https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c

Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
Signed-off-by: Du Cheng <ducheng2@gmail.com>
---
Detailed explanation:

In net/sched/sched_taprio.c:999
The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).

sched->cycle_time is accumulated within `parse_taprio_schedule()` during
`taprio_init()`, in the following 2 ways:

1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);

note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:

If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).

Reliable reproducable steps:
1. add net device team0 
2. add team_slave_0, team_slave_1
3. sendmsg(struct msghdr {
	.iov = struct nlmsghdr {
		.type = RTM_NEWQDISC,
	}
	struct tcmsg {
		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
		.nlattr[] = {
			TCA_KIND: "taprio",
			TCA_OPTIONS: {
				.nlattr = {
					TCA_TAPRIO_ATTR_PRIOMAP: ...,
					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
				}
			}
		}
	}
}

Callstack:

parse_taprio_schedule()
taprio_change()
taprio_init()
qdisc_create()
tc_modify_qdisc()
rtnetlink_rcv_msg()
...
sendmsg()

These steps are extracted from syzkaller reproducer:
https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000

 net/sched/sch_taprio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 8287894541e3..5f2ff0f15d5c 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
 	 * something went really wrong. In that case, we should warn about this
 	 * inconsistent state and return error.
 	 */
-	if (WARN_ON(!cycle))
+	if (!cycle)
 		return -EFAULT;
 
 	/* Schedule the start time for the beginning of the next
-- 
2.30.2

[PATCH v2] net: sched: tapr: remove WARN_ON() in taprio_get_start_time

[Du Cheng]

There is a reproducible sequence from the userland that will trigger a WARN_ON()
condition in taprio_get_start_time, which causes kernel to panic if configured
as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
userland-initiated syscalls.

Reported as bug on syzkaller:
https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c

Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
Signed-off-by: Du Cheng <ducheng2@gmail.com>
---
changelog:
v1: Discussion https://lore.kernel.org/netdev/YHfwUmFODUHx8G5W@carbon/T/
v2: fix typo


 net/sched/sch_taprio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 8287894541e3..33a829c1ba9b 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
 	 * something went really wrong. In that case, we should warn about this
 	 * inconsistent state and return error.
 	 */
-	if (WARN_ON(!cycle))
+	if (!cycle)
 		return -EFAULT;
 
 	/* Schedule the start time for the beginning of the next
-- 
2.30.2

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10243 bytes --]

Hi Du,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc7 next-20210414]
[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]

url:    https://github.com/0day-ci/linux/commits/Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7f75285ca572eaabc028cf78c6ab5473d0d160be
config: um-allmodconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
        git checkout 274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        # save the attached .config to linux build tree
        make W=1 W=1 ARCH=um 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

    1646 | static int taprio_init(struct Qdisc *sch, struct nlattr *opt,
         |            ^~~~~~~~~~~
   net/sched/sch_taprio.c:1712:29: error: invalid storage class for function 'taprio_queue_get'
    1712 | static struct netdev_queue *taprio_queue_get(struct Qdisc *sch,
         |                             ^~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1724:12: error: invalid storage class for function 'taprio_graft'
    1724 | static int taprio_graft(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1750:12: error: invalid storage class for function 'dump_entry'
    1750 | static int dump_entry(struct sk_buff *msg,
         |            ^~~~~~~~~~
   net/sched/sch_taprio.c:1780:12: error: invalid storage class for function 'dump_schedule'
    1780 | static int dump_schedule(struct sk_buff *msg,
         |            ^~~~~~~~~~~~~
   net/sched/sch_taprio.c:1816:12: error: invalid storage class for function 'taprio_dump'
    1816 | static int taprio_dump(struct Qdisc *sch, struct sk_buff *skb)
         |            ^~~~~~~~~~~
   net/sched/sch_taprio.c:1886:22: error: invalid storage class for function 'taprio_leaf'
    1886 | static struct Qdisc *taprio_leaf(struct Qdisc *sch, unsigned long cl)
         |                      ^~~~~~~~~~~
   net/sched/sch_taprio.c:1896:22: error: invalid storage class for function 'taprio_find'
    1896 | static unsigned long taprio_find(struct Qdisc *sch, u32 classid)
         |                      ^~~~~~~~~~~
   net/sched/sch_taprio.c:1905:12: error: invalid storage class for function 'taprio_dump_class'
    1905 | static int taprio_dump_class(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1917:12: error: invalid storage class for function 'taprio_dump_class_stats'
    1917 | static int taprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1931:13: error: invalid storage class for function 'taprio_walk'
    1931 | static void taprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
         |             ^~~~~~~~~~~
   net/sched/sch_taprio.c:1949:29: error: invalid storage class for function 'taprio_select_queue'
    1949 | static struct netdev_queue *taprio_select_queue(struct Qdisc *sch,
         |                             ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1956:12: error: initializer element is not constant
    1956 |  .graft  = taprio_graft,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1956:12: note: (near initialization for 'taprio_class_ops.graft')
   net/sched/sch_taprio.c:1957:11: error: initializer element is not constant
    1957 |  .leaf  = taprio_leaf,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1957:11: note: (near initialization for 'taprio_class_ops.leaf')
   net/sched/sch_taprio.c:1958:11: error: initializer element is not constant
    1958 |  .find  = taprio_find,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1958:11: note: (near initialization for 'taprio_class_ops.find')
   net/sched/sch_taprio.c:1959:11: error: initializer element is not constant
    1959 |  .walk  = taprio_walk,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1959:11: note: (near initialization for 'taprio_class_ops.walk')
   net/sched/sch_taprio.c:1960:11: error: initializer element is not constant
    1960 |  .dump  = taprio_dump_class,
         |           ^~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1960:11: note: (near initialization for 'taprio_class_ops.dump')
   net/sched/sch_taprio.c:1961:16: error: initializer element is not constant
    1961 |  .dump_stats = taprio_dump_class_stats,
         |                ^~~~~~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1961:16: note: (near initialization for 'taprio_class_ops.dump_stats')
   net/sched/sch_taprio.c:1962:18: error: initializer element is not constant
    1962 |  .select_queue = taprio_select_queue,
         |                  ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1962:18: note: (near initialization for 'taprio_class_ops.select_queue')
   net/sched/sch_taprio.c:1969:11: error: initializer element is not constant
    1969 |  .init  = taprio_init,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1969:11: note: (near initialization for 'taprio_qdisc_ops.init')
   net/sched/sch_taprio.c:1970:13: error: initializer element is not constant
    1970 |  .change  = taprio_change,
         |             ^~~~~~~~~~~~~
   net/sched/sch_taprio.c:1970:13: note: (near initialization for 'taprio_qdisc_ops.change')
   net/sched/sch_taprio.c:1971:13: error: initializer element is not constant
    1971 |  .destroy = taprio_destroy,
         |             ^~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1971:13: note: (near initialization for 'taprio_qdisc_ops.destroy')
   net/sched/sch_taprio.c:1972:12: error: initializer element is not constant
    1972 |  .reset  = taprio_reset,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1972:12: note: (near initialization for 'taprio_qdisc_ops.reset')
   net/sched/sch_taprio.c:1976:11: error: initializer element is not constant
    1976 |  .dump  = taprio_dump,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1976:11: note: (near initialization for 'taprio_qdisc_ops.dump')
   net/sched/sch_taprio.c:1981:19: error: initializer element is not constant
    1981 |  .notifier_call = taprio_dev_notifier,
         |                   ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1981:19: note: (near initialization for 'taprio_device_notifier.notifier_call')
   net/sched/sch_taprio.c:1984:19: error: invalid storage class for function 'taprio_module_init'
    1984 | static int __init taprio_module_init(void)
         |                   ^~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1994:20: error: invalid storage class for function 'taprio_module_exit'
    1994 | static void __exit taprio_module_exit(void)
         |                    ^~~~~~~~~~~~~~~~~~
   In file included from net/sched/sch_taprio.c:18:
   include/linux/module.h:129:42: error: invalid storage class for function '__inittest'
     129 |  static inline initcall_t __maybe_unused __inittest(void)  \
         |                                          ^~~~~~~~~~
   net/sched/sch_taprio.c:2000:1: note: in expansion of macro 'module_init'
    2000 | module_init(taprio_module_init);
         | ^~~~~~~~~~~
>> net/sched/sch_taprio.c:2000:1: warning: 'alias' attribute ignored [-Wattributes]
   In file included from net/sched/sch_taprio.c:18:
   include/linux/module.h:135:42: error: invalid storage class for function '__exittest'
     135 |  static inline exitcall_t __maybe_unused __exittest(void)  \
         |                                          ^~~~~~~~~~
   net/sched/sch_taprio.c:2001:1: note: in expansion of macro 'module_exit'
    2001 | module_exit(taprio_module_exit);
         | ^~~~~~~~~~~
   include/linux/module.h:135:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     135 |  static inline exitcall_t __maybe_unused __exittest(void)  \
         |  ^~~~~~
   net/sched/sch_taprio.c:2001:1: note: in expansion of macro 'module_exit'
    2001 | module_exit(taprio_module_exit);
         | ^~~~~~~~~~~
   net/sched/sch_taprio.c:2001:1: warning: 'alias' attribute ignored [-Wattributes]
   In file included from include/linux/module.h:21,
                    from net/sched/sch_taprio.c:18:
   include/linux/moduleparam.h:24:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      24 |  static const char __UNIQUE_ID(name)[]      \
         |  ^~~~~~
   include/linux/module.h:160:32: note: in expansion of macro '__MODULE_INFO'
     160 | #define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
         |                                ^~~~~~~~~~~~~
   include/linux/module.h:224:46: note: in expansion of macro 'MODULE_INFO'
     224 | #define MODULE_LICENSE(_license) MODULE_FILE MODULE_INFO(license, _license)
         |                                              ^~~~~~~~~~~
   net/sched/sch_taprio.c:2002:1: note: in expansion of macro 'MODULE_LICENSE'
    2002 | MODULE_LICENSE("GPL");
         | ^~~~~~~~~~~~~~
   net/sched/sch_taprio.c:2002:1: error: expected declaration or statement at end of input
   At top level:
   net/sched/sch_taprio.c:1130:32: warning: 'taprio_offload_get' defined but not used [-Wunused-function]
    1130 | struct tc_taprio_qopt_offload *taprio_offload_get(struct tc_taprio_qopt_offload
         |                                ^~~~~~~~~~~~~~~~~~


vim +/alias +2000 net/sched/sch_taprio.c

5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1999  
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28 @2000  module_init(taprio_module_init);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24378 bytes --]

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5242 bytes --]

Hi Du,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc7 next-20210414]
[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]

url:    https://github.com/0day-ci/linux/commits/Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7f75285ca572eaabc028cf78c6ab5473d0d160be
config: powerpc64-randconfig-r014-20210415 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 6a18cc23efad410db48a3ccfc233d215de7d4cb9)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
        git checkout 274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/sched/sch_taprio.c:1012:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1031:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1053:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1077:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1106:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1117:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1132:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1145:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1171:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1187:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1208:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1233:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1269:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1310:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1385:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1411:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1432:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1601:1: error: function definition is not allowed here
   {
   ^
   net/sched/sch_taprio.c:1617:1: error: function definition is not allowed here
   {
   ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   20 errors generated.


vim +1012 net/sched/sch_taprio.c

5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1009  
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1010  static void setup_first_close_time(struct taprio_sched *q,
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1011  				   struct sched_gate_list *sched, ktime_t base)
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28 @1012  {
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1013  	struct sched_entry *first;
6ca6a6654225f3 Vinicius Costa Gomes 2019-04-29  1014  	ktime_t cycle;
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1015  
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1016  	first = list_first_entry(&sched->entries,
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1017  				 struct sched_entry, list);
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1018  
037be0374078e2 Vedang Patel         2019-06-25  1019  	cycle = sched->cycle_time;
6ca6a6654225f3 Vinicius Costa Gomes 2019-04-29  1020  
6ca6a6654225f3 Vinicius Costa Gomes 2019-04-29  1021  	/* FIXME: find a better place to do this */
6ca6a6654225f3 Vinicius Costa Gomes 2019-04-29  1022  	sched->cycle_close_time = ktime_add_ns(base, cycle);
6ca6a6654225f3 Vinicius Costa Gomes 2019-04-29  1023  
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1024  	first->close_time = ktime_add_ns(base, first->interval);
23bddf692d369c Jakub Kicinski       2019-04-17  1025  	taprio_set_budget(q, first);
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1026  	rcu_assign_pointer(q->current_entry, NULL);
a3d43c0d56f1b9 Vinicius Costa Gomes 2019-04-29  1027  }
5a781ccbd19e46 Vinicius Costa Gomes 2018-09-28  1028  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33881 bytes --]

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Eric Dumazet]

On 4/15/21 9:50 AM, Du Cheng wrote:
> Le Thu, Apr 15, 2021 at 08:56:09AM +0200, Eric Dumazet a écrit :
>>
>>
>> On 4/15/21 8:39 AM, Du Cheng wrote:
>>> There is a reproducible sequence from the userland that will trigger a WARN_ON()
>>> condition in taprio_get_start_time, which causes kernel to panic if configured
>>> as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
>>> userland-initiated syscalls.
>>>
>>> Reported as bug on syzkaller:
>>> https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c
>>>
>>> Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
>>> Signed-off-by: Du Cheng <ducheng2@gmail.com>
>>> ---
>>> Detailed explanation:
>>>
>>> In net/sched/sched_taprio.c:999
>>> The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
>>> comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).
>>>
>>> sched->cycle_time is accumulated within `parse_taprio_schedule()` during
>>> `taprio_init()`, in the following 2 ways:
>>>
>>> 1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
>>> 2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);
>>>
>>> note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:
>>>
>>> If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
>>> TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
>>> in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).
>>>
>>> Reliable reproducable steps:
>>> 1. add net device team0 
>>> 2. add team_slave_0, team_slave_1
>>> 3. sendmsg(struct msghdr {
>>> 	.iov = struct nlmsghdr {
>>> 		.type = RTM_NEWQDISC,
>>> 	}
>>> 	struct tcmsg {
>>> 		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
>>> 		.nlattr[] = {
>>> 			TCA_KIND: "taprio",
>>> 			TCA_OPTIONS: {
>>> 				.nlattr = {
>>> 					TCA_TAPRIO_ATTR_PRIOMAP: ...,
>>> 					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
>>> 					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
>>> 				}
>>> 			}
>>> 		}
>>> 	}
>>> }
>>>
>>> Callstack:
>>>
>>> parse_taprio_schedule()
>>> taprio_change()
>>> taprio_init()
>>> qdisc_create()
>>> tc_modify_qdisc()
>>> rtnetlink_rcv_msg()
>>> ...
>>> sendmsg()
>>>
>>> These steps are extracted from syzkaller reproducer:
>>> https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000
>>>
>>>  net/sched/sch_taprio.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
>>> index 8287894541e3..5f2ff0f15d5c 100644
>>> --- a/net/sched/sch_taprio.c
>>> +++ b/net/sched/sch_taprio.c
>>> @@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
>>>  	 * something went really wrong. In that case, we should warn about this
>>>  	 * inconsistent state and return error.
>>>  	 */
>>> -	if (WARN_ON(!cycle))
>>> +	if (!cycle) {
>>>  		return -EFAULT;
>>>  
>>>  	/* Schedule the start time for the beginning of the next
>>>
>>
>> This 'fix' is wrong, not even compiled, thus not tested.
>>
>>  sched->cycle_time MUST not be zero ever, there are plenty
>> of other places where other crashes will happen.
>>
>> You are silencing a condition that should be caught much earlier.
>>
>> There is even a fat comment to explain this, that can be partially seen in your patch.
>>
>>
>> Correct fix will probably be :
>>
>> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
>> index 8287894541e3ce5f290be2e592c0dcbdf2ec6b60..189c617a582a2eecc92e35187379d4c2889289df 100644
>> --- a/net/sched/sch_taprio.c
>> +++ b/net/sched/sch_taprio.c
>> @@ -901,6 +901,8 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb,
>>  
>>                 list_for_each_entry(entry, &new->entries, list)
>>                         cycle = ktime_add_ns(cycle, entry->interval);
>> +               if (!cycle)
>> +                       return -EINVAL;
>>                 new->cycle_time = cycle;
>>         }
>>  
>>
>>
> 
> Hi Eric,
> 
> Sorry for the typo in the previous PATCH, I must have let the '{' slip out
> , when I tried to cleanup my own debugging lines.
> 
> Anyway, my intention was indeed to silently return error back to the userland,
> if the cycle is 0. The removal of WARN_ON() is so that this would not cause the
> kernel to panic.
> 
> `cycle` can be 0 if the userland provides invalid input, which should be
> anticipated and gracefully handled, hence panic would be too dramatic.


No. Again your patch is completely wrong, even when typo is fixed.

> 
> With the removal of WARN_ON(), the original logic was not
> altered: return -EINVAL to the userland and abort subsequent steps.
> 
> I will later submit a v2 to correct my typo.
> Thank you for your input.
> 
> Regards,
> Du Cheng
> 

Re: [PATCH v2] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Eric Dumazet]

On 4/15/21 9:59 AM, Du Cheng wrote:
> There is a reproducible sequence from the userland that will trigger a WARN_ON()
> condition in taprio_get_start_time, which causes kernel to panic if configured
> as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
> userland-initiated syscalls.
> 
> Reported as bug on syzkaller:
> https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c
> 
> Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
> Signed-off-by: Du Cheng <ducheng2@gmail.com>
> ---
> Detailed explanation:
> 
> In net/sched/sched_taprio.c:999
> The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
> comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).
> 
> sched->cycle_time is accumulated within `parse_taprio_schedule()` during
> `taprio_init()`, in the following 2 ways:
> 
> 1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
> 2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);
> 
> note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:
> 
> If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
> TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
> in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).
> 
> Reliable reproducable steps:
> 1. add net device team0 
> 2. add team_slave_0, team_slave_1
> 3. sendmsg(struct msghdr {
> 	.iov = struct nlmsghdr {
> 		.type = RTM_NEWQDISC,
> 	}
> 	struct tcmsg {
> 		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
> 		.nlattr[] = {
> 			TCA_KIND: "taprio",
> 			TCA_OPTIONS: {
> 				.nlattr = {
> 					TCA_TAPRIO_ATTR_PRIOMAP: ...,
> 					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
> 					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
> 				}
> 			}
> 		}
> 	}
> }
> 
> Callstack:
> 
> parse_taprio_schedule()
> taprio_change()
> taprio_init()
> qdisc_create()
> tc_modify_qdisc()
> rtnetlink_rcv_msg()
> ...
> sendmsg()
> 
> These steps are extracted from syzkaller reproducer:
> https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000
> 
>  net/sched/sch_taprio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 8287894541e3..5f2ff0f15d5c 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
>  	 * something went really wrong. In that case, we should warn about this
>  	 * inconsistent state and return error.
>  	 */
> -	if (WARN_ON(!cycle))
> +	if (!cycle)
>  		return -EFAULT;
>  
>  	/* Schedule the start time for the beginning of the next
> 


NACK

I already gave feedback in v1 why this fix is not correct.

Re: [PATCH] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 15004 bytes --]

Hi Du,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc7 next-20210415]
[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]

url:    https://github.com/0day-ci/linux/commits/Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7f75285ca572eaabc028cf78c6ab5473d0d160be
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Du-Cheng/net-sched-tapr-remove-WARN_ON-in-taprio_get_start_time/20210415-144126
        git checkout 274f557f95031e6965d9bb0ee67fdc22f2eb9b3a
        # save the attached .config to linux build tree
        make W=1 W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/sched/sch_taprio.c:1646:12: error: invalid storage class for function 'taprio_init'
    1646 | static int taprio_init(struct Qdisc *sch, struct nlattr *opt,
         |            ^~~~~~~~~~~
   net/sched/sch_taprio.c:1712:29: error: invalid storage class for function 'taprio_queue_get'
    1712 | static struct netdev_queue *taprio_queue_get(struct Qdisc *sch,
         |                             ^~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1724:12: error: invalid storage class for function 'taprio_graft'
    1724 | static int taprio_graft(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1750:12: error: invalid storage class for function 'dump_entry'
    1750 | static int dump_entry(struct sk_buff *msg,
         |            ^~~~~~~~~~
   net/sched/sch_taprio.c:1780:12: error: invalid storage class for function 'dump_schedule'
    1780 | static int dump_schedule(struct sk_buff *msg,
         |            ^~~~~~~~~~~~~
   net/sched/sch_taprio.c:1816:12: error: invalid storage class for function 'taprio_dump'
    1816 | static int taprio_dump(struct Qdisc *sch, struct sk_buff *skb)
         |            ^~~~~~~~~~~
   net/sched/sch_taprio.c:1886:22: error: invalid storage class for function 'taprio_leaf'
    1886 | static struct Qdisc *taprio_leaf(struct Qdisc *sch, unsigned long cl)
         |                      ^~~~~~~~~~~
   net/sched/sch_taprio.c:1896:22: error: invalid storage class for function 'taprio_find'
    1896 | static unsigned long taprio_find(struct Qdisc *sch, u32 classid)
         |                      ^~~~~~~~~~~
   net/sched/sch_taprio.c:1905:12: error: invalid storage class for function 'taprio_dump_class'
    1905 | static int taprio_dump_class(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1917:12: error: invalid storage class for function 'taprio_dump_class_stats'
    1917 | static int taprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
         |            ^~~~~~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1931:13: error: invalid storage class for function 'taprio_walk'
    1931 | static void taprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
         |             ^~~~~~~~~~~
   net/sched/sch_taprio.c:1949:29: error: invalid storage class for function 'taprio_select_queue'
    1949 | static struct netdev_queue *taprio_select_queue(struct Qdisc *sch,
         |                             ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1956:12: error: initializer element is not constant
    1956 |  .graft  = taprio_graft,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1956:12: note: (near initialization for 'taprio_class_ops.graft')
   net/sched/sch_taprio.c:1957:11: error: initializer element is not constant
    1957 |  .leaf  = taprio_leaf,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1957:11: note: (near initialization for 'taprio_class_ops.leaf')
   net/sched/sch_taprio.c:1958:11: error: initializer element is not constant
    1958 |  .find  = taprio_find,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1958:11: note: (near initialization for 'taprio_class_ops.find')
   net/sched/sch_taprio.c:1959:11: error: initializer element is not constant
    1959 |  .walk  = taprio_walk,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1959:11: note: (near initialization for 'taprio_class_ops.walk')
   net/sched/sch_taprio.c:1960:11: error: initializer element is not constant
    1960 |  .dump  = taprio_dump_class,
         |           ^~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1960:11: note: (near initialization for 'taprio_class_ops.dump')
   net/sched/sch_taprio.c:1961:16: error: initializer element is not constant
    1961 |  .dump_stats = taprio_dump_class_stats,
         |                ^~~~~~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1961:16: note: (near initialization for 'taprio_class_ops.dump_stats')
   net/sched/sch_taprio.c:1962:18: error: initializer element is not constant
    1962 |  .select_queue = taprio_select_queue,
         |                  ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1962:18: note: (near initialization for 'taprio_class_ops.select_queue')
   net/sched/sch_taprio.c:1969:11: error: initializer element is not constant
    1969 |  .init  = taprio_init,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1969:11: note: (near initialization for 'taprio_qdisc_ops.init')
   net/sched/sch_taprio.c:1970:13: error: initializer element is not constant
    1970 |  .change  = taprio_change,
         |             ^~~~~~~~~~~~~
   net/sched/sch_taprio.c:1970:13: note: (near initialization for 'taprio_qdisc_ops.change')
   net/sched/sch_taprio.c:1971:13: error: initializer element is not constant
    1971 |  .destroy = taprio_destroy,
         |             ^~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1971:13: note: (near initialization for 'taprio_qdisc_ops.destroy')
   net/sched/sch_taprio.c:1972:12: error: initializer element is not constant
    1972 |  .reset  = taprio_reset,
         |            ^~~~~~~~~~~~
   net/sched/sch_taprio.c:1972:12: note: (near initialization for 'taprio_qdisc_ops.reset')
   net/sched/sch_taprio.c:1976:11: error: initializer element is not constant
    1976 |  .dump  = taprio_dump,
         |           ^~~~~~~~~~~
   net/sched/sch_taprio.c:1976:11: note: (near initialization for 'taprio_qdisc_ops.dump')
   net/sched/sch_taprio.c:1981:19: error: initializer element is not constant
    1981 |  .notifier_call = taprio_dev_notifier,
         |                   ^~~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1981:19: note: (near initialization for 'taprio_device_notifier.notifier_call')
   net/sched/sch_taprio.c:1984:19: error: invalid storage class for function 'taprio_module_init'
    1984 | static int __init taprio_module_init(void)
         |                   ^~~~~~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1994:20: error: invalid storage class for function 'taprio_module_exit'
    1994 | static void __exit taprio_module_exit(void)
         |                    ^~~~~~~~~~~~~~~~~~
   In file included from include/linux/build_bug.h:5,
                    from include/linux/bits.h:22,
                    from include/linux/bitops.h:6,
                    from include/linux/bitmap.h:8,
                    from include/linux/ethtool.h:16,
                    from net/sched/sch_taprio.c:9:
>> include/linux/compiler.h:226:46: error: initializer element is not constant
     226 |   __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
         |                                              ^
   include/linux/init.h:236:2: note: in expansion of macro '__ADDRESSABLE'
     236 |  __ADDRESSABLE(fn)
         |  ^~~~~~~~~~~~~
   include/linux/init.h:241:2: note: in expansion of macro '__define_initcall_stub'
     241 |  __define_initcall_stub(__stub, fn)   \
         |  ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/init.h:253:2: note: in expansion of macro '____define_initcall'
     253 |  ____define_initcall(fn,     \
         |  ^~~~~~~~~~~~~~~~~~~
   include/linux/init.h:259:2: note: in expansion of macro '__unique_initcall'
     259 |  __unique_initcall(fn, id, __sec, __initcall_id(fn))
         |  ^~~~~~~~~~~~~~~~~
   include/linux/init.h:261:35: note: in expansion of macro '___define_initcall'
     261 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
         |                                   ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:290:30: note: in expansion of macro '__define_initcall'
     290 | #define device_initcall(fn)  __define_initcall(fn, 6)
         |                              ^~~~~~~~~~~~~~~~~
   include/linux/init.h:295:24: note: in expansion of macro 'device_initcall'
     295 | #define __initcall(fn) device_initcall(fn)
         |                        ^~~~~~~~~~~~~~~
   include/linux/module.h:86:24: note: in expansion of macro '__initcall'
      86 | #define module_init(x) __initcall(x);
         |                        ^~~~~~~~~~
   net/sched/sch_taprio.c:2000:1: note: in expansion of macro 'module_init'
    2000 | module_init(taprio_module_init);
         | ^~~~~~~~~~~
   In file included from include/linux/printk.h:6,
                    from include/linux/kernel.h:16,
                    from include/linux/bitmap.h:10,
                    from include/linux/ethtool.h:16,
                    from net/sched/sch_taprio.c:9:
   net/sched/sch_taprio.c:2001:13: error: initializer element is not constant
    2001 | module_exit(taprio_module_exit);
         |             ^~~~~~~~~~~~~~~~~~
   include/linux/init.h:298:50: note: in definition of macro '__exitcall'
     298 |  static exitcall_t __exitcall_##fn __exit_call = fn
         |                                                  ^~
   net/sched/sch_taprio.c:2001:1: note: in expansion of macro 'module_exit'
    2001 | module_exit(taprio_module_exit);
         | ^~~~~~~~~~~
   include/linux/init.h:298:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     298 |  static exitcall_t __exitcall_##fn __exit_call = fn
         |  ^~~~~~
   include/linux/module.h:98:24: note: in expansion of macro '__exitcall'
      98 | #define module_exit(x) __exitcall(x);
         |                        ^~~~~~~~~~
   net/sched/sch_taprio.c:2001:1: note: in expansion of macro 'module_exit'
    2001 | module_exit(taprio_module_exit);
         | ^~~~~~~~~~~
   In file included from include/linux/module.h:21,
                    from net/sched/sch_taprio.c:18:
   include/linux/moduleparam.h:24:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      24 |  static const char __UNIQUE_ID(name)[]      \
         |  ^~~~~~
   include/linux/module.h:160:32: note: in expansion of macro '__MODULE_INFO'
     160 | #define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
         |                                ^~~~~~~~~~~~~
   include/linux/module.h:177:21: note: in expansion of macro 'MODULE_INFO'
     177 | #define MODULE_FILE MODULE_INFO(file, KBUILD_MODFILE);
         |                     ^~~~~~~~~~~
   include/linux/module.h:224:34: note: in expansion of macro 'MODULE_FILE'
     224 | #define MODULE_LICENSE(_license) MODULE_FILE MODULE_INFO(license, _license)
         |                                  ^~~~~~~~~~~
   net/sched/sch_taprio.c:2002:1: note: in expansion of macro 'MODULE_LICENSE'
    2002 | MODULE_LICENSE("GPL");
         | ^~~~~~~~~~~~~~
   net/sched/sch_taprio.c:2002:1: error: expected declaration or statement at end of input
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    from include/linux/bitmap.h:10,
                    from include/linux/ethtool.h:16,
                    from net/sched/sch_taprio.c:9:
   include/linux/export.h:100:20: warning: unused variable '__kstrtabns_taprio_offload_free' [-Wunused-variable]
     100 |  extern const char __kstrtabns_##sym[];     \
         |                    ^~~~~~~~~~~~
   include/linux/export.h:147:39: note: in expansion of macro '___EXPORT_SYMBOL'
     147 | #define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
         |                                       ^~~~~~~~~~~~~~~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:159:33: note: in expansion of macro '_EXPORT_SYMBOL'
     159 | #define EXPORT_SYMBOL_GPL(sym)  _EXPORT_SYMBOL(sym, "_gpl")
         |                                 ^~~~~~~~~~~~~~
   net/sched/sch_taprio.c:1156:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
    1156 | EXPORT_SYMBOL_GPL(taprio_offload_free);
         | ^~~~~~~~~~~~~~~~~
   include/linux/export.h:99:20: warning: unused variable '__kstrtab_taprio_offload_free' [-Wunused-variable]
      99 |  extern const char __kstrtab_##sym[];     \
         |                    ^~~~~~~~~~
   include/linux/export.h:147:39: note: in expansion of macro '___EXPORT_SYMBOL'
     147 | #define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
         |                                       ^~~~~~~~~~~~~~~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:159:33: note: in expansion of macro '_EXPORT_SYMBOL'


vim +226 include/linux/compiler.h

^1da177e4c3f415 Linus Torvalds 2005-04-16  217  
7290d58095712a8 Ard Biesheuvel 2018-08-21  218  /*
7290d58095712a8 Ard Biesheuvel 2018-08-21  219   * Force the compiler to emit 'sym' as a symbol, so that we can reference
7290d58095712a8 Ard Biesheuvel 2018-08-21  220   * it from inline assembler. Necessary in case 'sym' could be inlined
7290d58095712a8 Ard Biesheuvel 2018-08-21  221   * otherwise, or eliminated entirely due to lack of references that are
7290d58095712a8 Ard Biesheuvel 2018-08-21  222   * visible to the compiler.
7290d58095712a8 Ard Biesheuvel 2018-08-21  223   */
7290d58095712a8 Ard Biesheuvel 2018-08-21  224  #define __ADDRESSABLE(sym) \
33def8498fdde18 Joe Perches    2020-10-21  225  	static void * __section(".discard.addressable") __used \
563a02b0c9704f6 Josh Poimboeuf 2020-08-18 @226  		__UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
7290d58095712a8 Ard Biesheuvel 2018-08-21  227  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64635 bytes --]

Re: [PATCH v2] net: sched: tapr: remove WARN_ON() in taprio_get_start_time()

[Du Cheng]

Le Thu, Apr 15, 2021 at 08:02:45PM +0200, Eric Dumazet a écrit :
> 
> 
> On 4/15/21 9:59 AM, Du Cheng wrote:
> > There is a reproducible sequence from the userland that will trigger a WARN_ON()
> > condition in taprio_get_start_time, which causes kernel to panic if configured
> > as "panic_on_warn". Remove this WARN_ON() to prevent kernel from crashing by
> > userland-initiated syscalls.
> > 
> > Reported as bug on syzkaller:
> > https://syzkaller.appspot.com/bug?extid=d50710fd0873a9c6b40c
> > 
> > Reported-by: syzbot+d50710fd0873a9c6b40c@syzkaller.appspotmail.com
> > Signed-off-by: Du Cheng <ducheng2@gmail.com>
> > ---
> > Detailed explanation:
> > 
> > In net/sched/sched_taprio.c:999
> > The condition WARN_ON(!cycle) will be triggered if cycle == 0. Value of cycle
> > comes from sched->cycle_time, where sched is of type(struct sched_gate_list*).
> > 
> > sched->cycle_time is accumulated within `parse_taprio_schedule()` during
> > `taprio_init()`, in the following 2 ways:
> > 
> > 1. from nla_get_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
> > 2. (if zero) from parse_sched_list(..., tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST], ...);
> > 
> > note: tb is a map parsed from netlink attributes provided via sendmsg() from the userland:
> > 
> > If both two attributes (TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME,
> > TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST) contain 0 values or are missing, this will result
> > in sched->cycle_time == 0 and hence trigger the WARN_ON(!cycle).
> > 
> > Reliable reproducable steps:
> > 1. add net device team0 
> > 2. add team_slave_0, team_slave_1
> > 3. sendmsg(struct msghdr {
> > 	.iov = struct nlmsghdr {
> > 		.type = RTM_NEWQDISC,
> > 	}
> > 	struct tcmsg {
> > 		.tcm_ifindex = ioctl(SIOCGIFINDEX, "team0"),
> > 		.nlattr[] = {
> > 			TCA_KIND: "taprio",
> > 			TCA_OPTIONS: {
> > 				.nlattr = {
> > 					TCA_TAPRIO_ATTR_PRIOMAP: ...,
> > 					TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST: {0},
> > 					TCA_TAPRIO_ATTR_SCHED_CLICKID: 0,
> > 				}
> > 			}
> > 		}
> > 	}
> > }
> > 
> > Callstack:
> > 
> > parse_taprio_schedule()
> > taprio_change()
> > taprio_init()
> > qdisc_create()
> > tc_modify_qdisc()
> > rtnetlink_rcv_msg()
> > ...
> > sendmsg()
> > 
> > These steps are extracted from syzkaller reproducer:
> > https://syzkaller.appspot.com/text?tag=ReproC&x=15727cf1900000
> > 
> >  net/sched/sch_taprio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > index 8287894541e3..5f2ff0f15d5c 100644
> > --- a/net/sched/sch_taprio.c
> > +++ b/net/sched/sch_taprio.c
> > @@ -996,7 +996,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
> >  	 * something went really wrong. In that case, we should warn about this
> >  	 * inconsistent state and return error.
> >  	 */
> > -	if (WARN_ON(!cycle))
> > +	if (!cycle)
> >  		return -EFAULT;
> >  
> >  	/* Schedule the start time for the beginning of the next
> > 
> 
> 
> NACK
> 
> I already gave feedback in v1 why this fix is not correct.
> 

Hi Eric,

Oh It must be I did not read your email carefully, and misundertood. I am sorry.
I sent a V3 following your suggestion. Please help review. Thank you!

Regards,
DU Cheng