Re: general protection fault in tomoyo_socket_sendmsg_permission

[Shuah Khan]

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