Re: general protection fault in tomoyo_socket_sendmsg_permission

[Shuah Khan <skhan@linuxfoundation.org>]

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@i-love.sakura.ne.jp ,
>>>> 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@i-love.sakura.ne.jp .
>>
> 
> 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