From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 0F82D5A026D for ; Wed, 15 Nov 2023 06:34:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700026483; 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=33CqYa636f8UXMERKHvWlV+QRddzjZWdcyiBGJhK2jM=; b=SfBxBn+PlFcZVSYsWx/JkpXix3xLHS3vCkE3+hSWvY6N3MOcFqT9uFkj3XaSwuR4wEEc2V OjrSFnomtiYcc7b6FVDq5RnrcMrjPeoVfaWTMo0bA61SgMhxQfqxZT/SEoOVd/LBUyg5T+ sywl41fPr5mfnuI24btv6QaWWDdFB/c= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-369-gTaaMXMoMbubAXhehqCzXA-1; Wed, 15 Nov 2023 00:34:42 -0500 X-MC-Unique: gTaaMXMoMbubAXhehqCzXA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A9F51811E7E; Wed, 15 Nov 2023 05:34:41 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.5]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A7DD71C060AE; Wed, 15 Nov 2023 05:34:40 +0000 (UTC) Date: Wed, 15 Nov 2023 06:32:59 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH] cppcheck,valgrind: Don't pass NULL to recv() with MSG_TRUNC Message-ID: <20231115063259.2c7bc5b0@elisabeth> In-Reply-To: <20231115044124.1496698-1-david@gibson.dropbear.id.au> References: <20231115044124.1496698-1-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LGX3IDNSZBKFDV7W37IHGJJLLHSAPSMN X-Message-ID-Hash: LGX3IDNSZBKFDV7W37IHGJJLLHSAPSMN 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 Wed, 15 Nov 2023 15:41:24 +1100 David Gibson wrote: > Usually, of course, it's invalid to pass a NULL buffer to recv(). However, > it's acceptable when using MSG_TRUNC, because that suppresses actually > writing to the buffer. So, we pass NULL in tcp_sock_consume(). > > Unfortunately, checker tools aren't always aware of that special case: we > already have a suppression for cppcheck to cover this. valgrind-3.22.0 > (present in Fedora 39) has a similar problem, generating a spurious warning > here. I haven't tried valgrind 3.22 yet, but... do you happen to know why test/valgrind.supp doesn't cover this anymore? > We could generate another suppression for valgrind, however, it so happens > that we already have tcp_buf_discard ready to hand. If we pass this > instead of NULL it makes both cppcheck and valgrind happy. We're still > using the MSG_TRUNC flag, the kernel doesn't actually have to copy data, > so we should still have the performance benefits of it. I'm not enthusiastic about this, because using tcp_buf_discard there might tell an optimising compiler that it's useful to prefetch it. We would also pass the actual address of tcp_buf_discard to the kernel, and I'm not sure if this has further subtle implications on possible optimisations in the kernel implementation (even though as you said no data is actually copied). -- Stefano