netdev - Re: [PATCH v2] tipc: fix shutdown() of connectionless socket

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ed285b44-2b09-8116-bb9e-a3879f72eb4d@i-love.sakura.ne.jp>
Date:   Thu, 3 Sep 2020 23:24:34 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        syzbot+e36f41d207137b5d12f7@...kaller.appspotmail.com,
        jmaloy@...hat.com, ying.xue@...driver.com,
        syzkaller-bugs@...glegroups.com, kuba@...nel.org,
        netdev@...r.kernel.org, tipc-discussion@...ts.sourceforge.net,
        Wouter Verhelst <w@...r.be>
Subject: Re: [PATCH v2] tipc: fix shutdown() of connectionless socket

Hello, Parthasarathy.

I have a question regarding commit 6f00089c7372ba97 ("tipc: remove SS_DISCONNECTING state").
That commit added

	sk->sk_shutdown = SEND_SHUTDOWN;

into tipc_shutdown(). What is the reason you chose SEND_SHUTDOWN despite how == SHUT_RDWR ?

Since Wouter commented that NBD expects SOCK_STREAM sockets, I think that passing TIPC's
stream socket is legal. And I can trigger hung task warning using a reproducer shown below.

----------
#include <fcntl.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <linux/nbd.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
        const int fd = open("/dev/nbd0", 3);
        int fds[2] = { -1, -1 };
        alarm(5);
        socketpair(PF_TIPC, SOCK_STREAM, 0, fds);
        ioctl(fd, NBD_SET_SOCK, fds[0]);
        ioctl(fd, NBD_DO_IT, 0); /* To be interrupted by SIGALRM. */
        return 0;
}
----------

Applying a patch shown below solves the hung task warning, but I can't evaluate
the side effect of this patch, for I don't know why you chose SEND_SHUTDOWN and
how TIPC socket works. Can we apply this patch?

----------
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2771,10 +2771,7 @@ static int tipc_shutdown(struct socket *sock, int how)
 
 	trace_tipc_sk_shutdown(sk, NULL, TIPC_DUMP_ALL, " ");
 	__tipc_shutdown(sock, TIPC_CONN_SHUTDOWN);
-	if (tipc_sk_type_connectionless(sk))
-		sk->sk_shutdown = SHUTDOWN_MASK;
-	else
-		sk->sk_shutdown = SEND_SHUTDOWN;
+	sk->sk_shutdown = SHUTDOWN_MASK;
 
 	if (sk->sk_state == TIPC_DISCONNECTING) {
 		/* Discard any unreceived messages */
----------