linux-kernel - Re: [rfcomm_run] WARNING: CPU: 1 PID: 79 at kernel/sched/core.c:7156 __might

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141003115020.GG10583@worktop.programming.kicks-ass.net>
Date:	Fri, 3 Oct 2014 13:50:20 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Fengguang Wu <fengguang.wu@...el.com>,
	Jet Chen <jet.chen@...el.com>, Su Tao <tao.su@...el.com>,
	Yuanhan Liu <yuanhan.liu@...el.com>, LKP <lkp@...org>,
	linux-kernel@...r.kernel.org,
	Marcel Holtmann <marcel@...tmann.org>,
	Peter Hurley <peter@...leysoftware.com>
Subject: Re: [rfcomm_run] WARNING: CPU: 1 PID: 79 at kernel/sched/core.c:7156
 __might_sleep()

On Thu, Oct 02, 2014 at 10:10:20PM +0200, Oleg Nesterov wrote:
> You know, I already thought about the patch below for other reasons, it
> can probably simplify other users of kthread_should_stop(). Because this
> way we can rely on the signal checks in schedule(). (Just in case, the
> patch is not complete, see TODO).
> 
> As for rfcomm_run(), perhaps it can ise it too?
> 
> 	set_kthread_wants_signal(true);
> 
> 	add_wait_queue(&rfcomm_wq, &wait);
> 	for (;;) {
> 		// This is only possible if kthread_should_stop() == T

True because kthreads SIG_IGN everything, right?

> 		if (signal_pending(current))
> 			break;
> 
> 		rfcomm_process_sessions();
> 		wait_woken(TASK_INTERRUPTIBLE);
> 	}
> 
> Of course, this assumes that rfcomm_process_sessions() can't do something
> "really bad" if signal_pending() is true.

So from what I can think of, everything that does an INTERRUPTIBLE sleep
will 'malfunction' after that, right? Which might be quite a lot
actually.

> What do you think?

Interesting approach, but somewhat risky I tihnk, due to that
INTERRUPTIBLE thing.

> --- x/kernel/kthread.c
> +++ x/kernel/kthread.c
> @@ -49,6 +49,7 @@ struct kthread {
>  enum KTHREAD_BITS {
>  	KTHREAD_IS_PER_CPU = 0,
>  	KTHREAD_SHOULD_STOP,
> +	KTHREAD_WANTS_SIGNAL,
>  	KTHREAD_SHOULD_PARK,
>  	KTHREAD_IS_PARKED,
>  };
> @@ -442,6 +443,21 @@ int kthread_park(struct task_struct *k)
>  	return ret;
>  }
>  
> +void set_kthread_wants_signal(bool on)
> +{
> +	unsigned long *kflags = &to_kthread(current)->flags;
> +	unsigned long irqflags;
> +
> +	if (on) {
> +		set_bit(KTHREAD_WANTS_SIGNAL, kflags);
> +	} else {
> +		spin_lock_irqsave(&current->sighand->siglock, irqflags);
> +		clear_bit(KTHREAD_WANTS_SIGNAL, kflags);
> +		recalc_sigpending();
> +		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
> +	}
> +}
> +
>  /**
>   * kthread_stop - stop a thread created by kthread_create().
>   * @k: thread created by kthread_create().
> @@ -469,6 +485,9 @@ int kthread_stop(struct task_struct *k)
>  	if (kthread) {
>  		set_bit(KTHREAD_SHOULD_STOP, &kthread->flags);
>  		__kthread_unpark(k, kthread);
> +		// TODO: this is racy, we need ->siglock.
> +		if (test_bit(KTHREAD_WANTS_SIGNAL, &to_kthread(k)->flags))
> +			 set_tsk_thread_flag(k, TIF_SIGPENDING);

Right, but taking that should not really be a problem afaict, this is a
slow path if ever there was one.

>  		wake_up_process(k);
>  		wait_for_completion(&kthread->exited);
>  	}
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/