From nobody Fri Oct 13 16:37:47 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4S6XHL5w0Fz4wStd; Fri, 13 Oct 2023 16:37:50 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4S6XHL5P7zz4CYM; Fri, 13 Oct 2023 16:37:50 +0000 (UTC) (envelope-from kp@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697215070; 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=tbq9ZTRESpmdnSz07j4FO0DVm3Ca5eBhkdvjIGPW/L8=; b=ITnoBP+yNnml/BVH4Z+fOLu7luP93Q0dx4eZcez4wr3QqozjfJKZuqRK9riEDfX1p40OYn q10H/pEszIM3sCvddZjxLGOWuWaRTW9jt8vFkubGHbxF/SgyIjL1G0BgPqv3FzRIiLNwVX BSW9WZof+/mI3Hrzh6x9xzWPlOtAo7qRVRoUNGmPHIHfY1a3JiVjD10QBDG9SNNwl7JGgz rNq0zUCu3x0Fh6znuroufUhiyqra3S3u4Z9uUcX7sUFkrwQk3X7EfTn6XFco7p5qvYWIw0 E7DI2ZTH19J44Fz3JD/XyLYb4l9NQe5QS94PpIGl8lTcCFamV4rvMyl+fmhf6w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1697215070; a=rsa-sha256; cv=none; b=DXWt/UIA2k5ZciS2k9yAeuQqKSQcLC8uzfgqngRnFWpYiqcPsLgMarU5SV6pNBXcmkFjlv YtWFAL0V3GP478ZnIard1Oq/2A5koW0xMXS9NKp1BbJhpeu8jmUg2nwP6KalZdJ0XASoqq woV5AbXah+BgcsRkJkB9bLO8FKuy+eLe31Jpi+QuUlLQs5hI8ZuO3bH1UEKSZKc5+gcfHp LBPWaYOBgj63AsmsZyTovHprsQqN2uyhrd66PA7OZo0uDQkZYsX+rrSRs3151vFe+0eGqs c9waYXQy0xyJWnpdVvQcvYsj6yJpUCbXLf69H15oejDJ0qFV/omGLcGlPxoiMA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697215070; 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=tbq9ZTRESpmdnSz07j4FO0DVm3Ca5eBhkdvjIGPW/L8=; b=XyGe+hV8Ago42+6M7qHqvBpNsM6TTJWjdH/SuxdTzcYUYRmMrhmD/62aFeSxQh4/n9bLUk c/uiNv8r9ihje627COtBaLmMeTsAYiKGoZ2w9JiK20vcz53Tr4wvIE+haWoxcODadzIW/O 3M4w1WgpA4bph4FeTeYOiTCFAHLiRUbiVsc5HAIDXBpSMOJXHGoxTUpCyDv/iyTcd99R3T r/0K+d8bSAs/wJqepwGVJ7OuaMvEY7mzLp/D1lYJS5rnP5PXIcG/jiJy1tR2urCw1VImx3 jdpgK3BWdXQCpSF4kNlzdf1zX4K6igYFBTxA6y22Gaxj6bTkDOc2FxslOUGhkQ== Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "mx1.codepro.be", Issuer "R3" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 4S6XHL3k2Rz1MRG; Fri, 13 Oct 2023 16:37:50 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: by venus.codepro.be (Postfix, authenticated sender kp) id 44CCD3DF27; Fri, 13 Oct 2023 18:37:48 +0200 (CEST) From: Kristof Provost To: "Alexander V. Chernikov" Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 5f19f790b392 - main - netlink: add snl(3) support for parsing unknown-size arrays Date: Fri, 13 Oct 2023 18:37:47 +0200 X-Mailer: MailMate (1.14r5937) Message-ID: <8CFADE61-3604-4915-9AE2-F52E6C17DB6E@FreeBSD.org> In-Reply-To: <202305271114.34RBED8O007212@gitrepo.freebsd.org> References: <202305271114.34RBED8O007212@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 27 May 2023, at 13:14, Alexander V. Chernikov wrote: > The branch main has been updated by melifaro: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D5f19f790b3923354871ce049= b84c7807ecb0cad5 > > commit 5f19f790b3923354871ce049b84c7807ecb0cad5 > Author: Alexander V. Chernikov > AuthorDate: 2023-05-27 11:09:01 +0000 > Commit: Alexander V. Chernikov > CommitDate: 2023-05-27 11:13:14 +0000 > > netlink: add snl(3) support for parsing unknown-size arrays > > Reviewed by: bapt > Differential Review: https://reviews.freebsd.org/D40282 > MFC after: 2 weeks > --- > static inline bool > snl_attr_get_nla(struct snl_state *ss __unused, struct nlattr *nla, > const void *arg __unused, void *target) > @@ -878,6 +959,7 @@ snl_realloc_msg_buffer(struct snl_writer *nw, size_= t sz) > memcpy(new_base, nw->base, nw->offset); > if (nw->hdr !=3D NULL) { > int hdr_off =3D (char *)(nw->hdr) - nw->base; > + > nw->hdr =3D (struct nlmsghdr *)(void *)((char *)new_base + hdr_off);= I=E2=80=99ve been working on converting a few more pf ioctls to using net= link, and ran into an odd failure once the request got big enough. My req= uests all resulted in =E2=80=9CNo buffer space available=E2=80=9D. That t= urned out to be because they were of size zero. Some more digging showed = that the struct nlmsghdr *hdr pointer returned by snl_finalize_msg() was = different from the one returned by snl_create_msg_request(). I believe the issue is that we=E2=80=99re re-allocating the header here, = but I was still using the old struct nlmsghdr *hdr in the calling functio= n. That seems to be a pattern in other netlink code as well, e.g. https://cg= it.reebsd.org/src/tree/sbin/route/route_netlink.c#n269 where we get the s= truct nlmsghdr *hdr when we create the request. We seem to be getting away with that here, because most requests are smal= l. Still, I think we should at the very least fix that so it doesn=E2=80=99= t mislead others. Ideally I think we should avoid returning pointers that will unexpectedly= be invalidated. That is, I think we shouldn=E2=80=99t allow callers to a= ccess struct snl_writer.hdr in the first place. (Also, I don=E2=80=99t immediately see where we free the old memory. Are = we leaking that or am I missing something?) Best regards, Kristof