Re: [PATCH] io_uring/net: save msghdr->msg_control for retries

Stefan Metzmacher
2023-06-19T13:09:41Z

Am 19.06.23 um 15:05 schrieb Jens Axboe:
> On 6/19/23 3:57?AM, Stefan Metzmacher wrote:
>> Hi Jens,
>>
>>> If the application sets ->msg_control and we have to later retry this
>>> command, or if it got queued with IOSQE_ASYNC to begin with, then we
>>> need to retain the original msg_control value. This is due to the net
>>> stack overwriting this field with an in-kernel pointer, to copy it
>>> in. Hitting that path for the second time will now fail the copy from
>>> user, as it's attempting to copy from a non-user address.
>>
>> I'm not 100% sure about the impact of this change.
>>
>> But I think the logic we need is that only the
>> first __sys_sendmsg_sock() that returns > 0 should
>> see msg_control. A retry because of MSG_WAITALL should
>> clear msg_control[len] for a follow up __sys_sendmsg_sock().
>> And I fear the patch below would not clear it...
>>
>> Otherwise the receiver/socket-layer will get the same msg_control twice,
>> which is unexpected.
>
> Yes agree, if we do transfer some (but not all) data and WAITALL is set,
> it should get cleared. I'll post a patch for that.

Thanks!

> Note that it was also broken before, just differently broken. The most
> likely outcome here was a full retry and now getting -EFAULT.

Yes, I can see that it was broken before...

metze