linux-kernel - Re: general protection fault in tomoyo_socket_sendmsg

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-next>] [day] [month] [year] [list]

Message-ID: <5f71e0c1-d387-6d72-d8e4-edb11cf57f72@linuxfoundation.org>
Date:   Fri, 13 Nov 2020 10:14:40 -0700
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     Hillf Danton <hdanton@...a.com>,
        syzbot <syzbot+95ce4b142579611ef0a9@...kaller.appspotmail.com>
Cc:     linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Valentina Manea <valentina.manea.m@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        syzkaller-bugs@...glegroups.com,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: general protection fault in tomoyo_socket_sendmsg_permission

On 11/13/20 5:00 AM, Hillf Danton wrote:
> Thu, 12 Nov 2020 23:21:26 -0800
>> syzbot found the following issue on:
>>
>> HEAD commit:    9dbc1c03 Merge tag 'xfs-5.10-fixes-3' of git://git.kernel...
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=10453034500000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=1735b7978b1c3721
>> dashboard link: https://syzkaller.appspot.com/bug?extid=95ce4b142579611ef0a9
>> compiler:       gcc (GCC) 10.1.0-syz 20200507
>> userspace arch: i386
>>
>> Unfortunately, I don't have any reproducer for this issue yet.
>>

I would like to see the reproducer for this. I just can't reproduce
this problem.

> 
> Fix 96c2737716d5 ("usbip: move usbip kernel code out of staging")
> by closing the race between readers and writer of ud->tcp_socket on
> sock shutdown. To do that, add waitqueue for usbip device and make
> writer wait for all readers to go home before releasing the socket.
> 
> 
> --- a/drivers/usb/usbip/usbip_common.h
> +++ b/drivers/usb/usbip/usbip_common.h
> @@ -277,6 +277,9 @@ struct usbip_device {
>   		void (*reset)(struct usbip_device *);
>   		void (*unusable)(struct usbip_device *);
>   	} eh_ops;
> +
> +	atomic_t sk_shutdown_count;
> +	wait_queue_head_t sk_shutdown_waitq;
>   };
>   
>   #define kthread_get_run(threadfn, data, namefmt, ...)			   \
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1008,15 +1008,20 @@ static void vhci_shutdown_connection(str
>   
>   	/* kill threads related to this sdev */
>   	if (vdev->ud.tcp_rx) {
> +		atomic_inc(&ud->sk_shutdown_count);
>   		kthread_stop_put(vdev->ud.tcp_rx);
>   		vdev->ud.tcp_rx = NULL;
>   	}
>   	if (vdev->ud.tcp_tx) {
> +		atomic_inc(&ud->sk_shutdown_count);
>   		kthread_stop_put(vdev->ud.tcp_tx);
>   		vdev->ud.tcp_tx = NULL;
>   	}
>   	pr_info("stop threads\n");
>   
> +	wait_event(ud->sk_shutdown_waitq,
> +			!atomic_read(&ud->sk_shutdown_count));
> +
>   	/* active connection is closed */
>   	if (vdev->ud.tcp_socket) {
>   		sockfd_put(vdev->ud.tcp_socket);
> @@ -1104,6 +1109,8 @@ static void vhci_device_init(struct vhci
>   	vdev->ud.eh_ops.reset = vhci_device_reset;
>   	vdev->ud.eh_ops.unusable = vhci_device_unusable;
>   
> +	init_waitqueue_head(&vdev->ud.sk_shutdown_waitq);
> +
>   	usbip_start_eh(&vdev->ud);
>   }
>   
> --- a/drivers/usb/usbip/vhci_rx.c
> +++ b/drivers/usb/usbip/vhci_rx.c
> @@ -264,5 +264,8 @@ int vhci_rx_loop(void *data)
>   		vhci_rx_pdu(ud);
>   	}
>   
> +	if (atomic_dec_and_test(&ud->sk_shutdown_count))
> +		wake_up(&ud->sk_shutdown_waitq);
> +
>   	return 0;
>   }
> --- a/drivers/usb/usbip/vhci_tx.c
> +++ b/drivers/usb/usbip/vhci_tx.c
> @@ -252,5 +252,8 @@ int vhci_tx_loop(void *data)
>   		usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
>   	}
>   
> +	if (atomic_dec_and_test(&ud->sk_shutdown_count))
> +		wake_up(&ud->sk_shutdown_waitq);
> +
>   	return 0;
>   }
> 


Without a reproducer, it is hard to verify the fix.

thanks,
-- Shuah