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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 27 Jun 2007 16:24:37 +0400 From: Oleg Nesterov <oleg@...sign.ru> To: Satyam Sharma <satyam.sharma@...il.com> Cc: Jeff Layton <jlayton@...hat.com>, Herbert Xu <herbert@...dor.apana.org.au>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, "Eric W. Biederman" <ebiederm@...ssion.com>, linux-cifs-client@...ts.samba.org Subject: Re: [PATCH] RFC: have tcp_recvmsg() check kthread_should_stop() and treat it as if it were signalled On 06/27, Satyam Sharma wrote: > > Thanks for your comments, I'm still not convinced, however. An perhaps you are right. I don't have a very strong opinion on that. Still I can't understand why it is better if kthread_stop() sends a signal as well. Contrary, I believe we should avoid signals when it comes to kernel threads. One can always use force_sig() or allow_signal() + send_sig() when it is really needed, like cifs does. > On 6/26/07, Oleg Nesterov <oleg@...sign.ru> wrote: > > > >Personally, I don't think we should do this. > > > >kthread_stop() doesn't always mean "kill this thread asap". Suppose that > >CPU_DOWN does kthread_stop(workqueue->thread) but doesn't flush the queue > >before that (we did so before 2.6.22 and perhaps we will do again). Now > >work_struct->func() doing tcp_recvmsg() or wait_event_interruptible() > >fails, > >but this is probably not that we want. > > Anyway, I think _all_ usages of kthread_stop() in the kernel *do* want > the thread to stop *right then*. After all, kthread_stop() doesn't even > return (gets blocked on wait_for_completion()) till it knows the target > kthread *has* exited completely. Yes, kthread_stop(k) means that k should exit eventually, but I don't think that kthread_stop() should try to force the exit. > And if a workqueue is blocked on tcp_recvmsg() or skb_recv_datagram() > or some such, I don't see how that flush_workqueue (if that is what you > meant) would succeed anyway (unless you do send the signal too), timeout, but this was just a silly example. I am talking about the case when wait_event_interruptible() should not fail (unless something bad happens) inside the "while (!kthread_should_stop())" loop. Note also that kthread could use TASK_INTERRUPTIBLE sleep because it doesn't want to contribute to loadavg, and because it knows that all signals are ignored. > Note that the exact scenario you're talking about wouldn't mean the > kthread getting killed before it's supposed to be stopped anyway. Yes sure, we can't kill the kernel thread via signal. I meant we can have some unexpected failure. > >(offtopic) > > > > cifs_mount: > > > > send_sig(SIGKILL,srvTcp->tsk,1); > > tsk = srvTcp->tsk; > > if(tsk) > > kthread_stop(tsk); > > > >This "if(tsk)" looks wrong to me. > > I think it's bogus myself. [ Added linux-cifs-client@...ts.samba.org to Cc: > ] > > >Can srvTcp->tsk be NULL? If yes, send_sig() > >is not safe. Can srvTcp->tsk become NULL after send_sig() ? If yes, this > >check is racy, and kthread_stop() is not safe. > > That's again something the atomicity I proposed above could avoid? I think this "if(tsk)" is just bogus, and should be killed. Oleg. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists