ACK: [SRU focal/linux-oem-5.6 1/1] io_uring: fix missing msg_name assignment

[Krzysztof Kozlowski]

On 14/05/2021 09:59, Thadeu Lima de Souza Cascardo wrote:
> On Fri, May 14, 2021 at 09:43:19AM -0400, Krzysztof Kozlowski wrote:
>> On 14/05/2021 08:14, Thadeu Lima de Souza Cascardo wrote:
>>> From: Pavel Begunkov <asml.silence at gmail.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1928028
>>>
>>> Ensure to set msg.msg_name for the async portion of send/recvmsg,
>>> as the header copy will copy to/from it.
>>>
>>> Cc: stable at vger.kernel.org # v5.5+
>>> Signed-off-by: Pavel Begunkov <asml.silence at gmail.com>
>>> Signed-off-by: Jens Axboe <axboe at kernel.dk>
>>> (backported from commit dd821e0c95a64b5923a0c57f07d3f7563553e756)
>>> [cascardo: missing commit 52de1fe12240, there is no io_recvmsg_copy_hdr]
>>> [cascardo: also, recvmsg paths do not set msg_name when copying msg,
>>>  but use uaddr]
>>
>> Do you refer here to the io_recvmsg_prep() and you state it does not
>> need setting msg_name?
> 
> The original commit dd821e0c95a6 sets msg.name at io_recvmsg_copy_hdr, which is
> not present in 5.6, and was introduced in commit 52de1fe12240. Which is,
> indeed, called at io_recvmsg_prep and io_recvmsg. Though msg_name is set on
> io_recvmsg, it is not necessary. And so, not necessary at io_recvmsg_prep
> either.
> 
> Running a test with IOSQE_ASYNC flag and IORING_OP_RECVMSG, and a msg_name,
> there is no such issue as the one we found out with IORING_OP_SENDMSG. Simply
> because, as I stated, a uaddr is used.
> 
> From net/socket.c:copy_msghdr_from_user, save_addr is used to signal this is
> called from recvmsg, not sendmsg:
> 
>         if (save_addr)
>                 *save_addr = msg.msg_name;
> 
>         if (msg.msg_name && kmsg->msg_namelen) {
>                 if (!save_addr) {
>                         err = move_addr_to_kernel(msg.msg_name,
>                                                   kmsg->msg_namelen,
>                                                   kmsg->msg_name);
>                         if (err < 0)
>                                 return err;
>                 }
> 
> Then, on net/socket.c:____sys_recvmsg:
> 
> 	struct sockaddr_storage addr;
> [...]
>         if (uaddr != NULL) {
>                 err = move_addr_to_user(&addr,
>                                         msg_sys->msg_namelen, uaddr,
>                                         uaddr_len);
>                 if (err < 0)
>                         goto out;
>         }
> 
> So, there is no need to set msg_name on the kmsg on recvmsg paths, but a uaddr
> must be given, where a pointer to the user given msg_name will be saved and
> used to copy the kernel stack-allocated address to.
> 
> Makes sense?

Yes, thanks!


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at canonical.com>


Best regards,
Krzysztof