Re: [PATCH] sched: Do not bug in __sched_setscheduler() when pi is not used

[Florian Fainelli]

On 11/19/18 8:35 AM, Steven Rostedt wrote:
> On Mon, 19 Nov 2018 17:24:32 +0100
> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> 
>> On Mon, Nov 19, 2018 at 10:46:54AM -0500, Steven Rostedt wrote:
>>> On Mon, 19 Nov 2018 16:13:11 +0100
>>> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>>>   
>>>>> Can this patch also be applied to the stable trees? The offending commit
>>>>> was first introduced in 4.2.    
>>>>
>>>> What is the git commit id of this patch in Linus's tree?  
>>>
>>> 896bbb2522587e3b8eb2a0d204d43ccc1042a00d
>>>
>>> The subject was changed when it was applied.  
>>
>> Ah, that helps.
>>
>> But why is this really needed in the older kernels?  You want to crash
>> your machine if someone got things wrong?  Given that I doubt this is
>> being hit anymore, why is it needed in 4.9.y and 4.4.y?
> 
> The problem is that it can crash when people didn't get it wrong (by
> using sysrq).
> 
> The bug happened when we combined two functions into one, and where the
> BUG_ON() from one was now added to the other function. The one (where
> pi is true) requires the function to be called with interrupts
> disabled. The other did not have this requirement (and pi happens to be
> false). Thus, you can trigger the BUG_ON(in_interrupt()) with a simple
> sysrq key stroke, and crash your kernel when it did nothing wrong.
> 
>>
>> Also, it doesn't apply there so someone needs to do the backport...
>>
> 
> I did the change against v4.2.8 below.

Thanks Steven! Here is the local 4.9 backport:

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 917be221438b..6b3fff6a6437 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4087,8 +4087,8 @@ static int __sched_setscheduler(struct task_struct *p,
        int queue_flags = DEQUEUE_SAVE | DEQUEUE_MOVE;
        struct rq *rq;

-       /* may grab non-irq protected spin_locks */
-       BUG_ON(in_interrupt());
+       /* The pi code expects interrupts enabled */
+       BUG_ON(pi && in_interrupt());
 recheck:
        /* double check policy once rq lock held */
        if (policy < 0) {


> 
> -- Steve
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 6776631676e0..b2af7989eb5b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3682,7 +3682,7 @@ static int __sched_setscheduler(struct task_struct *p,
>  	int reset_on_fork;
>  
>  	/* may grab non-irq protected spin_locks */
> -	BUG_ON(in_interrupt());
> +	BUG_ON(pi && in_interrupt());
>  recheck:
>  	/* double check policy once rq lock held */
>  	if (policy < 0) {
> 


-- 
Florian