From nobody Sun Oct 23 18:18:54 2022 X-Original-To: freebsd-virtualization@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 4MwRKv6BN8z4fspn for ; Sun, 23 Oct 2022 18:18:59 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-io1-xd33.google.com (mail-io1-xd33.google.com [IPv6:2607:f8b0:4864:20::d33]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4MwRKv26Ycz3xG6; Sun, 23 Oct 2022 18:18:59 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-io1-xd33.google.com with SMTP id p16so6174734iod.6; Sun, 23 Oct 2022 11:18:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :sender:from:to:cc:subject:date:message-id:reply-to; bh=+D4+WWMvRO8oVr+nhxWSK05crQatbw1UaQ2jpzmttsU=; b=ai+qW95Som8Ii8af0hg68OdpaZEs+ZDDSF4pIPL96UbM4P+2EaO5/9YouwemPyZtel +RzGurgx2mR2hcMdBxMp/xoXm69xXLt3g6kxv3VEkUInYi4aglV5F87G0mmo5BAEPNgs gA6ujbGNlG6r/DT4FB3Cat1XYYUELKNHrcIpVs2XFmE80pnxD0hOQiZZaCWxbnwsZomz NfM840SUDoGrVdspef3EivpcdE3MEszQZm004lWBFh6s6qKADm0+9T7ved67RDqYaBd6 0iv8B4aR2J/fPcPnLTeGpKJGpTSrOV7176biZzpCxbM0i+1cog/hoF4Ube37PUCMbPWl CqEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :sender:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+D4+WWMvRO8oVr+nhxWSK05crQatbw1UaQ2jpzmttsU=; b=6LzEa87D3fwV7bcJ4xPMErAG9UJLTIpLN+p3FxyOgjbV9gsL4A1J1HAL9BNt2rh2aP Zh5m7IFIw2kLpbqT67XhXdWceUfafnMEHq+PavPRzJK2BSIAojTSxy2EnH+1kMfjQJD2 Hpsh9qpbc8sAsB8E4hKDfw6FzokDz6dZnZ2Qb2/mf7cIttIakXuBYOf+tT4oI5iVplpd iuGBryJ9x1fWPDO+uWIrNytJXCDwohweKJiJ/pVhUnkPLbZ6RYJ9Vu6FjoatRphiVFDz WNzqJkDxdOnlDhihjRpFy4T/p9X0Mf8hi8BxGSEQw1LbVTQF/5RESN6W+fyORfkIFvYF HxMA== X-Gm-Message-State: ACrzQf1qaqC8lLRWKlrbWySCfrRFQNCs96gBASh8ooP3LOFRO/TDER4r p6LLksp/FYQxhM/gP7gN21IAm5/VGhU= X-Google-Smtp-Source: AMsMyM5VeJUbjEqMD3G/8xZHF6vETYgPD7BjaKRKz08Nhcb92KXqkJIbK8rz3p6Xa0mPJXy/s2Ackw== X-Received: by 2002:a05:6602:13c8:b0:68a:db5d:269d with SMTP id o8-20020a05660213c800b0068adb5d269dmr18449964iov.209.1666549137464; Sun, 23 Oct 2022 11:18:57 -0700 (PDT) Received: from nuc (192-0-220-237.cpe.teksavvy.com. [192.0.220.237]) by smtp.gmail.com with ESMTPSA id o124-20020a022282000000b00363cb638bffsm6394556jao.179.2022.10.23.11.18.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Oct 2022 11:18:56 -0700 (PDT) Date: Sun, 23 Oct 2022 14:18:54 -0400 From: Mark Johnston To: jhb@freebsd.org, freebsd-virtualization@freebsd.org Subject: bhyve nvlist constness bug Message-ID: List-Id: Discussion List-Archive: https://lists.freebsd.org/archives/freebsd-virtualization List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-virtualization@freebsd.org X-BeenThere: freebsd-virtualization@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Rspamd-Queue-Id: 4MwRKv26Ycz3xG6 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b=ai+qW95S; dmarc=none; spf=pass (mx1.freebsd.org: domain of markjdb@gmail.com designates 2607:f8b0:4864:20::d33 as permitted sender) smtp.mailfrom=markjdb@gmail.com X-Spamd-Result: default: False [-2.67 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.97)[-0.972]; MID_RHS_NOT_FQDN(0.50)[]; FORGED_SENDER(0.30)[markj@freebsd.org,markjdb@gmail.com]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20210112]; MIME_GOOD(-0.10)[text/plain]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::d33:from]; FREEMAIL_ENVFROM(0.00)[gmail.com]; MLMMJ_DEST(0.00)[freebsd-virtualization@freebsd.org]; MIME_TRACE(0.00)[0:+]; RCPT_COUNT_TWO(0.00)[2]; RCVD_TLS_LAST(0.00)[]; FROM_NEQ_ENVFROM(0.00)[markj@freebsd.org,markjdb@gmail.com]; FROM_HAS_DN(0.00)[]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DMARC_NA(0.00)[freebsd.org]; TO_DN_NONE(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-ThisMailContainsUnwantedMimeParts: N I'm going through compiler warnings in the bhyve code with the aim of bumping WARNS, since the current setting hides bugs. There's one in the config code that looks a bit tricky to resolve and I was hoping for some guidance on the right way to do that. The basic problem is _lookup_config_node() may return an nvlist via nvlist_get_nvlist(), but nvlist_get_nvlist() returns a const nvlist_t *, so _lookup_config_node() is discarding the const qualifier. And indeed, some callers will modify the returned node. The use of nvlist_move_nvlist() in _lookup_config_node() has a similar problem: nvlist_move_* "consumes" the passed value and is not supposed to be mutated after. I'm pretty sure this is actually a non-issue with our nvlist implementation; when adding an nvlist value to an nvlist, it doesn't store anything except for the pointer itself, so you can mutate it safely. Note, this is not true for all value types, specifically arrays. However, there are multiple nvlist implementations out there, and ours might change such that bhyve's config code becomes incorrect. The bug seems annoying to fix because consumers can do this: nvlist_t *nvl = find_config_node(path); set_config_value_node(nvl, "foo", "bar"); So, if we naively changed find_config_node() to return a copy of the nvlist at "path", set_config_value_node() would have to somehow figure out where in the config tree to insert the updated nvlist, but it doesn't have the path anymore. To fix it properly could change find_config_node() to return some opaque type which contains the source path of the node so that we can DTRT when mutating the node. IMO it's better if the config node type is opaque to consumers. Or, make each nvlist node store its source path by storing it in the nvlist itself. e.g., the nvlist node describing the device model at "pci.0.1.2" would have a "__path" key with value "pci.0.1.2", so that set_config_value_node() can figure out where in the tree to replace an existing copy of "pci.0.1.2". Or is there a simpler way to fix this that I missed? Any thoughts? I'm happy to implement the suggestions above but didn't want to do that work without some buy-in.