From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=R5G7be8f; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id D02225A004C for ; Tue, 26 Nov 2024 06:14:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1732598096; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hzw2EZ1TKPxXcPsOfvMWuomVADTyQySFXkoME1DoBuE=; b=R5G7be8foqp3cm2td70PcObs42KdKgDkuX40t3gAWL0LummgsyP5EKovXKQo9gzTvkLz9j p31o3susGZvaep4rimAfQhWC940igKZ9/9kurf3LzHyaW5dFUohNzvQKrTqMf69rchcWh9 ssypCTvA02b28TjdJOBkAogVelg6wZU= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-274-tkmCcVsKNYK28CL_DfXVjQ-1; Tue, 26 Nov 2024 00:14:54 -0500 X-MC-Unique: tkmCcVsKNYK28CL_DfXVjQ-1 X-Mimecast-MFC-AGG-ID: tkmCcVsKNYK28CL_DfXVjQ Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-434a6483514so2566665e9.0 for ; Mon, 25 Nov 2024 21:14:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732598092; x=1733202892; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=hzw2EZ1TKPxXcPsOfvMWuomVADTyQySFXkoME1DoBuE=; b=bmrI5LmWHPYKCmA5i+BLSBlXqJPUOooua9lGm9RamSqh/E4wefyUkh5SjbsHNTtl1A frbEynS/mNXIyCGbBt+/KJxJUegBgyZZ8ehleA9nauA7tLzR5DmjGV+AFUfGMU2P1Dfg SMouvF75Iyj0jZZ6sM2lI2RMLBUuFSqId6ckhrb4yP74dKHZwu/7UQTlh9Fsx+P9E/zo uqofESTJobesCYV0uU3FHVJutm0BreiR9+nFpoSR8D7FTk4OHQ1AvcNVE9MEheMYiCCz 6wHbGgHL6O1BHSjxP9Mt4YJ3PuqGyjxmMOY0V6E9KmgcaNFuWo6cG8NQGrWsN2kpriYl +CeQ== X-Gm-Message-State: AOJu0YxZTcMPm91oVLOvsrC74jlQQh/JWuDeD/RVbmYhXyxhC+t6Q2rn 9LPFYQUEbyxCV1Ve2IGtSt5gIH29hrNHL/TePtAng8BoRd62uCpmJ0XmR/ARQA+ZKIYKHSdgkjV dI+Ami0Jg8ILMMlTaWBmMAkJTllCm9U6Fn1crr56Wf7fJwGMbbedPu0ZuQY4Ql960VmA5OK/Rtw tJdNmbPliQDrfKwbz+Zgsiz4DLwsWYrSJy X-Gm-Gg: ASbGncs6BKmmt0QIQcYOX8MQ5yHPmn/rOHP0fjdaFm7lMdUIm9I/o8SyxJBKEJHrMZm pLban4+7yjq50Ot2HKAWAXl0SbMJ4HOriKuwawRhY7jO9PBy0GklNmmiUrLvVJHljw2IGwrjlo1 Gm6ZoO0zqxiV8fnoXUHizHUXwSBD/etzoUWpPhmuZ6fu6RTYhEwNe69aVU8P9nOcVGRvlBG1wNo 5lG4yWGtd6gPbV+piKm4pbAut5Y5LEEy8SNfs+scZKl3FeoHwnt3Ut+cPhlbE8RoSxQom33dN5J yJg= X-Received: by 2002:a05:600c:3b91:b0:434:a29d:6c71 with SMTP id 5b1f17b1804b1-434a29d6d6bmr30112675e9.27.1732598092105; Mon, 25 Nov 2024 21:14:52 -0800 (PST) X-Google-Smtp-Source: AGHT+IHSAoouB1+Z+nWExMH/5o5BwqCUxooUiFDxDDpbbM4MPkMPqvMkAgAyoR8WlI8UXeE5dxDl/g== X-Received: by 2002:a05:600c:3b91:b0:434:a29d:6c71 with SMTP id 5b1f17b1804b1-434a29d6d6bmr30112565e9.27.1732598091726; Mon, 25 Nov 2024 21:14:51 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3825fbc3a47sm12481063f8f.73.2024.11.25.21.14.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Nov 2024 21:14:50 -0800 (PST) Date: Tue, 26 Nov 2024 06:14:43 +0100 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH v14 7/9] vhost-user: add vhost-user Message-ID: <20241126061443.3287f86d@elisabeth> In-Reply-To: <20241122164337.3377854-8-lvivier@redhat.com> References: <20241122164337.3377854-1-lvivier@redhat.com> <20241122164337.3377854-8-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Jx_hQVHtO4WAAqJrVBOVKTg2tzwEUQLR8FsWVJ-OnBI_1732598093 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: IFTG7WVBN5N4TPQ2DY24RTJWSATOK7LC X-Message-ID-Hash: IFTG7WVBN5N4TPQ2DY24RTJWSATOK7LC X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, 22 Nov 2024 17:43:34 +0100 Laurent Vivier wrote: > +/** > + * tcp_vu_data_from_sock() - Handle new data from socket, queue to vhost-user, > + * in window > + * @c: Execution context > + * @conn: Connection pointer > + * > + * Return: Negative on connection reset, 0 otherwise > + */ > +int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) > +{ > + uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap; > + struct vu_dev *vdev = c->vdev; > + struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE]; > + const struct flowside *tapside = TAPFLOW(conn); > + size_t fillsize, hdrlen; > + int v6 = CONN_V6(conn); > + uint32_t already_sent; > + const uint16_t *check; > + int i, iov_cnt; > + ssize_t len; > + > + if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) { > + debug("Got packet, but RX virtqueue not usable yet"); > + return 0; > + } > + > + already_sent = conn->seq_to_tap - conn->seq_ack_from_tap; > + > + if (SEQ_LT(already_sent, 0)) { > + /* RFC 761, section 2.1. */ > + flow_trace(conn, "ACK sequence gap: ACK for %u, sent: %u", > + conn->seq_ack_from_tap, conn->seq_to_tap); > + conn->seq_to_tap = conn->seq_ack_from_tap; > + already_sent = 0; > + if (tcp_set_peek_offset(conn->sock, 0)) { > + tcp_rst(c, conn); > + return -1; > + } > + } > + > + if (!wnd_scaled || already_sent >= wnd_scaled) { > + conn_flag(c, conn, STALLED); > + conn_flag(c, conn, ACK_FROM_TAP_DUE); > + return 0; > + } > + > + /* Set up buffer descriptors we'll fill completely and partially. */ > + > + fillsize = wnd_scaled - already_sent; > + > + /* collect the buffers from vhost-user and fill them with the > + * data from the socket > + */ > + len = tcp_vu_sock_recv(c, conn, v6, already_sent, fillsize, &iov_cnt); > + if (len < 0) { > + if (len != -EAGAIN && len != -EWOULDBLOCK) { > + tcp_rst(c, conn); > + return len; > + } > + return 0; > + } > + > + if (!len) { > + if (already_sent) { > + conn_flag(c, conn, STALLED); > + } else if ((conn->events & (SOCK_FIN_RCVD | TAP_FIN_SENT)) == > + SOCK_FIN_RCVD) { > + int ret = tcp_vu_send_flag(c, conn, FIN | ACK); > + if (ret) { > + tcp_rst(c, conn); > + return ret; > + } > + > + conn_event(c, conn, TAP_FIN_SENT); > + } > + > + return 0; > + } > + > + conn_flag(c, conn, ~STALLED); > + > + /* Likely, some new data was acked too. */ > + tcp_update_seqack_wnd(c, conn, false, NULL); > + > + /* initialize headers */ > + /* iov_vu is an array of buffers and the buffer size can be > + * smaller than the frame size we want to use but with > + * num_buffer we can merge several virtio iov buffers in one packet > + * we need only to set the packet headers in the first iov and > + * num_buffer to the number of iov entries > + */ > + > + hdrlen = tcp_vu_hdrlen(v6); > + for (i = 0, check = NULL; i < head_cnt; i++) { > + struct iovec *iov = &elem[head[i]].in_sg[0]; > + int buf_cnt = head[i + 1] - head[i]; > + int dlen = iov_size(iov, buf_cnt) - hdrlen; Unless I'm missing something, to me this looks like a false positive, but Coverity now reports, for this line: (17) Event function_return: Function "iov_size(iov, buf_cnt)" returns 0. (18) Event overflow_const: Expression "iov_size(iov, buf_cnt) - hdrlen", which is equal to 18446744073709551550, where "iov_size(iov, buf_cnt)" is known to be equal to 0, and "hdrlen" is known to be equal to 66, underflows the type that receives it, an unsigned integer 64 bits wide. ...I don't think iov_size() can ever return 0 if we reach this point, but I would try to cover this by either, in order of preference: 1. not sending this frame if iov_size(iov, buf_cnt) < hdrlen 2. an ASSERT(iov_size(iov, buf_cnt) >= hdrlen) It can be a follow-up patch, there's no need to re-post the whole thing (at least not just for this), unless you see something that actually needs to be fixed. -- Stefano