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

[Laurent Vivier]

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 <del...@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