Re: general protection fault in tomoyo_socket_sendmsg_permission

[Shuah Khan]

On 2/11/21 9:58 PM, Tetsuo Handa wrote:
> On 2021/02/12 11:22, Tetsuo Handa wrote:
> > On 2021/02/12 10:34, Shuah Khan wrote:
> > > On 2/10/21 6:14 PM, Tetsuo Handa wrote:
> > > > (Dropping LSM ML because this is not a TOMOYO's bug.)
> > > >
> > > > On 2021/02/11 4:29, Shuah Khan wrote:
> > > > > This is a good find. I already replied to the thread to send a complete
> > > > > fix.
> > > >
> > > > As I said at https://lkml.kernel.org/r/f8cae6b1-8f84-0e6a-7d9c-fc4aec68f07b@xxxxxxxxxxxxxxxxxxx ,
> > > > the as-is patch is effectively a complete fix. And applying the as-is patch should help spending
> > > > syzbot resources for reproducing "general protection fault in tomoyo_socket_sendmsg_permission"
> > > > with debug printk() patch applied, which in turn will help you in
> > > >
> > > > > Right. I would like to get a clear understanding of how this condition
> > > > > is triggered. I am not saying this isn't a problem. Understanding how
> > > > > it is triggered helps find the best fix.
> > > >
> > > > part. Therefore, I strongly expect you to apply this version now.
> > >
> > > Is there a reproducer for this problem?
> >
> > There is no reproducer for "general protection fault in tomoyo_socket_sendmsg_permission" problem, but
> > the race condition is explained at https://lkml.kernel.org/r/676d4518-0faa-9fab-15db-0db8d216d7fb@xxxxxxxxxxxxxxxxxxx .
>
> Here is a race window widening patch, and I locally reproduced "general protection fault in tomoyo_socket_sendmsg_permission".
>
> diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h
> index 8be857a4fa13..a7c68097aa1d 100644
> --- a/drivers/usb/usbip/usbip_common.h
> +++ b/drivers/usb/usbip/usbip_common.h
> @@ -286,6 +286,8 @@ struct usbip_device {
>   	if (!IS_ERR(__k)) {						   \
>   		get_task_struct(__k);					   \
>   		wake_up_process(__k);					   \
> +	} else {							   \
> +		__k = NULL;						   \
>   	}								   \
>   	__k;								   \
>   })
> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
> index be37aec250c2..93e1271d0f5d 100644
> --- a/drivers/usb/usbip/vhci_sysfs.c
> +++ b/drivers/usb/usbip/vhci_sysfs.c
> @@ -389,8 +389,12 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
>   	/* end the lock */
>   
>   	vdev->ud.tcp_rx = kthread_get_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
> -	vdev->ud.tcp_tx = kthread_get_run(vhci_tx_loop, &vdev->ud, "vhci_tx");
> +	{
> +		struct task_struct *tx = kthread_get_run(vhci_tx_loop, &vdev->ud, "vhci_tx");
>   
> +		schedule_timeout_uninterruptible(HZ);
> +		vdev->ud.tcp_tx = tx;
> +	}
>   	rh_port_connect(vdev, speed);
>   
>   	return count;

Thank you. This is helpful. I will try to reproduce the problem with
this test code.

thanks,
-- Shuah