From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756001AbbJ1O5d (ORCPT ); Wed, 28 Oct 2015 10:57:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60712 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754668AbbJ1O5c (ORCPT ); Wed, 28 Oct 2015 10:57:32 -0400 Date: Wed, 28 Oct 2015 16:53:53 +0100 From: Oleg Nesterov To: Markus Pargmann Cc: Andrew Morton , nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH -mm 2/3] nbd: nbd_thread_recv: remove the buggy kernel_dequeue_signal() Message-ID: <20151028155353.GC22672@redhat.com> References: <20151025152625.GA1385@redhat.com> <20151025152639.GA1402@redhat.com> <20151026074417.GE16521@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151026074417.GE16521@pengutronix.de> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/26, Markus Pargmann wrote: > > Hi Oleg, > > On Sun, Oct 25, 2015 at 04:26:39PM +0100, Oleg Nesterov wrote: > > nbd_thread_recv() is called by userspace, it is very wrong to dequeue > > and throw out a signal. > > This signal handling for a userspace process is implicitly implemented > for several years already through the timeout handling. This is nothing > new and could potentially break userspace if someone disconnects NBD > using the kill command. As we expose the appropriate PID of the process > as well this is possible to be used in an init script. > > So I am not sure about this patch yet. I strongly believe this kernel_dequeue_signal() must die, it is very wrong and I can't even enumerate all problems. And why do you want it? Probably to "hide" the SIGKILL sent while this process was exposed as ->task_recv. This can not work even in the simplest case when this process is single-threaded. kernel_dequeue_signal() won't clear SIGNAL_GROUP_EXIT set by SIGKILL, it won't remove SIGKILL from shared_pending if it was sent by the kill command. Note also that SIGKILL can be sent from another thread which does exec/group_exit/coredump. In this case the state of this thread group will be very wrong after kernel_dequeue_signal() removes SIGKILL from task_struct->pending. Finally. We can not even know which signal we are going to dequeue and throw out. Say, it can be SIGCHLD or any other unrelated signal. No, this can't be right. Oleg.