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

Florian Fainelli
2018-11-19T19:10:02Z

On 11/19/18 8:35 AM, Steven Rostedt wrote:
> On Mon, 19 Nov 2018 17:24:32 +0100
> Greg KH <gregkh@linuxfoundation.org> 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@linuxfoundation.org> 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