Re: [PATCH -mm 2/3] nbd: nbd_thread_recv: remove the buggy kernel_dequeue_signal()

[Markus Pargmann]

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 do not understand why nbd_thread_recv() (and nbd_thread_send() btw)
> does sock_shutdown(); the caller, __nbd_ioctl(NBD_DO_IT) does this too.

Yes indeed. This has to be fixed as well, thanks.

Best Regards,

Markus


> 
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>  drivers/block/nbd.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index e5d96e5..0ffd73c 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -444,9 +444,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
>  	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
>  
>  	if (signal_pending(current)) {
> -		ret = kernel_dequeue_signal(NULL);
> -		dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal %d\n",
> -			 task_pid_nr(current), current->comm, ret);
> +		dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal",
> +			 task_pid_nr(current), current->comm);
>  		mutex_lock(&nbd->tx_lock);
>  		sock_shutdown(nbd);
>  		mutex_unlock(&nbd->tx_lock);
> -- 
> 1.5.5.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature