Re: [PATCH 2/2] linux-user: Allow sendmsg() without IOV

[Laurent Vivier <laurent@vivier.eu>]

Le 31/01/2023 à 14:50, Helge Deller a écrit :
> On 1/31/23 14:34, Helge Deller wrote:
>> On 1/31/23 13:28, Laurent Vivier wrote:
>>> Le 12/12/2022 à 18:34, Helge Deller a écrit :
>>>> Applications do call sendmsg() without any IOV, e.g.:
>>>>   sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=NULL, msg_iovlen=0,
>>>>              msg_control=[{cmsg_len=36, cmsg_level=SOL_ALG, cmsg_type=0x2}],
>>>>              msg_controllen=40, msg_flags=0}, MSG_MORE) = 0
>>>>   sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="The quick brown fox jumps over 
>>>> t"..., iov_len=183}],
>>>>              msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_ALG, cmsg_type=0x3}],
>>>>              msg_controllen=24, msg_flags=0}, 0) = 183
>>>>
>>>> The function do_sendrecvmsg_locked() is used for sndmsg() and recvmsg()
>>>> and calls lock_iovec() to lock the IOV into memory. For the first
>>>> sendmsg() above it returns NULL and thus wrongly skips the call the host
>>>> sendmsg() syscall, which will break the calling application.
>>>>
>>>> Fix this issue by:
>>>> - allowing sendmsg() even with empty IOV
>>>> - skip recvmsg() if IOV is NULL
>>>> - skip both if the return code of do_sendrecvmsg_locked() != 0, which
>>>>    indicates some failure like EFAULT on the IOV
>>>>
>>>> Tested with the debian "ell" package with hppa guest on x86_64 host.
>>>>
>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>> ---
>>>>   linux-user/syscall.c | 9 +++++++--
>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index a365903a3a..9e2c0a18fc 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -3330,7 +3330,10 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp,
>>>>                        target_vec, count, send);
>>>>       if (vec == NULL) {
>>>>           ret = -host_to_target_errno(errno);
>>>> -        goto out2;
>>>> +        /* allow sending packet without any iov, e.g. with MSG_MORE flag */
>>>
>>> why don't you check only for count is 0?
>>> Somehing like:
>>>
>>> if (vec == NULL && (count || !send)) {
>>
>> You mean:
>> if (vec == NULL && (!count || !send)) {
>>      goto out2;
>>
>> lock_iovec() sets errno=0 if count==0, but for invalid addresses it sets errno!=0,
>> so my current check (for which ret is the same as errno):
>>          if (!send || ret) {
>>              goto out2;
>> exits on memfaults too, while a check for just count==0 would succeed.
> 
> forget the above...
> Maybe just checking for count==0 would be sufficient. I don't know yet if
> there are any other corner cases where replacing the check of
>            if (!send || ret) {
> with
>            if (!send || count==0) {
> would behave different.
> Checking against errno which is reported back from lock_iovec() seemed more
> logical for me. This patch has been as-is in the buildd trees now since weeks
> without any problems so far at least.
> 

It seems kernel also checks for the invalid address even with count=0, so I'm going to apply this 
patch as-is.

Thanks,
Laurent