linux-kernel - Re: [PATCH -mm 2/3] nbd: nbd_thread_recv: remove the buggy kernel_dequeue

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: <20151026074417.GE16521@pengutronix.de>
Date:	Mon, 26 Oct 2015 08:44:17 +0100
From:	Markus Pargmann <mpa@...gutronix.de>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	nbd-general@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -mm 2/3] nbd: nbd_thread_recv: remove the buggy
 kernel_dequeue_signal()

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@...hat.com>
> ---
>  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 |

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)