Date: Fri, 05 Mar 2021 09:32:12 +0100 From: "Kristof Provost" <kp@FreeBSD.org> To: "John Baldwin" <jhb@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: a079e38b08f2 - main - ossl: Add Poly1305 digest support. Message-ID: <4EB11ECA-1788-49D2-9362-09D955C8D695@FreeBSD.org> In-Reply-To: <202103032321.123NLOKb093507@gitrepo.freebsd.org> References: <202103032321.123NLOKb093507@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 4 Mar 2021, at 0:21, John Baldwin wrote: > The branch main has been updated by jhb: > > URL: = > https://cgit.FreeBSD.org/src/commit/?id=3Da079e38b08f2f07c50ba915dae66d= 099559abdcc > > commit a079e38b08f2f07c50ba915dae66d099559abdcc > Author: John Baldwin <jhb@FreeBSD.org> > AuthorDate: 2021-03-03 23:17:29 +0000 > Commit: John Baldwin <jhb@FreeBSD.org> > CommitDate: 2021-03-03 23:20:57 +0000 > > ossl: Add Poly1305 digest support. > > Reviewed by: cem > Sponsored by: Netflix > Differential Revision: https://reviews.freebsd.org/D28754 It looks like this broke the LINT builds: linking kernel ld: error: duplicate symbol: Poly1305_Final >>> defined at ossl_poly1305.c >>> ossl_poly1305.o:(Poly1305_Final) >>> defined at xform_poly1305.c >>> xform_poly1305.o:(.text+0xA0) ld: error: duplicate symbol: Poly1305_Init >>> defined at ossl_poly1305.c >>> ossl_poly1305.o:(Poly1305_Init) >>> defined at xform_poly1305.c >>> xform_poly1305.o:(.text+0x0) ld: error: duplicate symbol: Poly1305_Update >>> defined at ossl_poly1305.c >>> ossl_poly1305.o:(Poly1305_Update) >>> defined at xform_poly1305.c >>> xform_poly1305.o:(.text+0x60) ld: warning: common OPENSSL_ia32cap_P is overridden ld: warning: common OPENSSL_ia32cap_P is overridden ld: warning: common OPENSSL_ia32cap_P is overridden ld: warning: common OPENSSL_ia32cap_P is overridden ld: warning: common OPENSSL_ia32cap_P is overridden *** [kernel] Error code 1 (See also = https://ci.freebsd.org/job/FreeBSD-main-aarch64-LINT/6074/console ) Best regards Kristof From owner-dev-commits-src-all@freebsd.org Fri Mar 5 08:41:59 2021 Return-Path: <owner-dev-commits-src-all@freebsd.org> Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 2169A552FA8; Fri, 5 Mar 2021 08:41:59 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DsLnf4K4lz3qSS; Fri, 5 Mar 2021 08:41:58 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id 1258ftJG058855; Fri, 5 Mar 2021 00:41:55 -0800 (PST) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id 1258ftai058854; Fri, 5 Mar 2021 00:41:55 -0800 (PST) (envelope-from freebsd) From: "Rodney W. Grimes" <freebsd@gndrsh.dnsmgr.net> Message-Id: <202103050841.1258ftai058854@gndrsh.dnsmgr.net> Subject: Re: git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2. In-Reply-To: <5c413c07-ad22-01e1-ee45-35fbc04a4875@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Date: Fri, 5 Mar 2021 00:41:55 -0800 (PST) CC: Warner Losh <imp@bsdimp.com>, Brandon Bergren <bdragon@freebsd.org>, "Rodney W. Grimes" <rgrimes@freebsd.org>, Ed Maste <emaste@freebsd.org>, src-committers <src-committers@freebsd.org>, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 4DsLnf4K4lz3qSS X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository <dev-commits-src-all.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/dev-commits-src-all>, <mailto:dev-commits-src-all-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/dev-commits-src-all/>; List-Post: <mailto:dev-commits-src-all@freebsd.org> List-Help: <mailto:dev-commits-src-all-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all>, <mailto:dev-commits-src-all-request@freebsd.org?subject=subscribe> X-List-Received-Date: Fri, 05 Mar 2021 08:41:59 -0000 > On 3/3/21 5:25 PM, Warner Losh wrote: > > > > > > On Wed, Mar 3, 2021 at 10:21 AM Nathan Whitehorn > > <nwhitehorn@freebsd.org <mailto:nwhitehorn@freebsd.org>> wrote: > > > > > > > > On 3/3/21 11:53 AM, Warner Losh wrote: > > > > > > > [clipping non-technical pre-history] > > > > > > Thanks. re-reading it now, I think I was more grumpy than warranted. > > And for that I apologize. Thanks for omitting it from the rest of the > > thread. > > No worries, this happens, especially with the pandemic. I know I've > definitely been more prickly this year than normal... > > > > > > >? ? ?The installer *does* mount the partition in advance, so checking > > >? ? ?whether > > >? ? ?there is a mounted file system is a perfectly reasonable test to > > >? ? ?do. We > > >? ? ?could also check fstab. I would like to understand what is > > actually > > >? ? ?wrong here first, though. Especially after this misfire -- > > which is > > >? ? ?problematic for reasons that are still not clear to me, since > > >? ? ?there are > > >? ? ?a number of standard directories in hier(7) not in mtree -- > > I want to > > >? ? ?make sure we actually do have consensus about what is > > changing and > > >? ? ?why. > > > > > > > > > At the top level, we default to having directories in mtree unless > > > there's a good reason not to. We disagree as to whether the > > installer > > > should take the presence or absence of the directory as a strong > > > enough reason to do something. I don't think that's a good reason. > > > > > > But leaving that aside, let's say we?wanted to reuse the install > > boot > > > part of the installer to update boot blocks as part of > > installworld. > > > If we can talk through that example w/o it in mtree, then we can > > leave > > > it out. The last time I worked through this, though, I thought I'd > > > talked myself into needing it. > > > > > Looking at bootconfig, we could use machdep.bootmethod to > > determine if > > > we need to update the ESP. If we didn't use that, then the ESP > > > shouldn't be touched. This is, at the moment, x86 centric, but > > could > > > trivially be added to architectures (I'm happy to add it). This > > would > > > prevent the 'false positive' that's possible in cases where we've > > > installed UEFI then downgraded to BIOS because of some problem > > (though > > > purely in the context of the installer, I guess this isn't an > > issue). > > > Even with your approach, we'd bogusly update an ESP (though one > > could > > > argue you might want that). We could also change the code so that > > > 'unsupported' architectures just didn't update. This is why I think > > > it's a bit fragile to rely only on the directory being present. It > > > should have something mounted there. If you wanted to mkfs_dos?+ > > mkdir > > > efi at the top level, you could check for that directory if you > > were > > > looking for a flag, though that would still update on a BIOS > > boot the > > > ESP, and prevent false positives if run as part of an update. > > > > I think we would *want* to update an ESP that is mounted but not > > currently being used. If I set up a dual BIOS/EFI-boot system for > > some > > reason and happened to install an update while booted from BIOS, I > > would > > be deeply astonished if my configured-by-the-installer EFI bootloader > > did not also get updated. > > > > > > Yea, it's unclear to me what POLA here is, to be honest. Some of that > > is driven by a deep desire not to accidentally update USB drives that > > have a bootable image on them as well, so that may overly color my > > thinking. > > Agreed on all counts here. > > > (As an aside, I would also much rather the installer use an update > > utility to set up the ESP than have the update utility use the > > installer.) > > > > > > Agreed. We can work towards that after the release. It would be better > > if we could accumulate the scripts from a number of different places, > > find a good way to make them callable from those places more easily > > and start to move that tribal knowledge back into the base system > > where it belongs, imho. Baptiste raised an important point years ago > > that we also need to think about doing that with a way to 'plug in' > > $NEWEST_CLOUD's packages, containers, layout such that they could > > provide the details and then the automation would just work with them > > too: image building, release customization, boot block update, etc. > > > > So here's a proposal, now that everyone is in the CC list: > > - We add /boot/efi back to mtree, even though I find it kind of > > weird to > > have it there I think we're too close to the release to have a > > conclusion on this. > > - We have the installer check for either the ESP directory being an > > active mountpoint or being in the in-progress fstab, whichever is > > easiest to implement (they are equivalent for the installer). > > > > > > I'm OK with both of these points. If others are opposed to the first > > one, I'm willing to see how people react to it in the upgrade path > > before changing it again. We should get closure on Ed's proposed > > change here. I think it's good and should go in right after your > > changes. I'd start on your changes, and give people until the morning > > to pipe up with any objections. > Here's a patch to do this: > https://reviews.freebsd.org/D29068 > > It takes several hours to do the full test of building world, building > release ISOs, and running them through qemu, so it will be a while yet > before I feel comfortable committing. But it's a two-line diff and the > pieces worked independently, so the chances it works are pretty high. > Comments appreciated. > > > If that seems OK, I'll post another review for the change. > > > > > A long-term project I've had has been to try to update the boot > > blocks > > > as part of installworld or maybe as part of installboot. We have > > > really poor reuse as a project in this area. Every little > > > orchestration thing wrote its own thing, and all of them have > > done it > > > badly. I was hoping to be able to reuse this code, or modify the > > > installer to use whatever we come up with there. As part of that, I > > > had talked myself into thinking we always needed /boot/efi, but I'm > > > having trouble reconstructing why that is now though I know it > > had to > > > do with installed systems and bootstrapping issues... I know I was > > > worried about questions about 'why isn't /boot/efi on the system by > > > default so I can mount it' for people that have upgraded, but I > > recall > > > there was more to it than that. With it in mtree, an installworld > > > (even w/o an ESP update) would create it and people could mount > > it w/o > > > having to mkdir which they might make as $SOMETHING_ELSE. So I > > guess > > > that's a bit of a weak reason to absolutely require it in mtree. > > > > Thanks a lot for the explanation. I'm agreed entirely about the > > problem > > and the difficulty -- hopefully this set of changes helps at least. > > > > > > It does. It starts to get people to use the same mount point for the > > ESP and we can then constrain the problem a bit and where we can't > > constrain it we can parameterize it. > > > > As for mtree, I was imagining this as something like /home, which > > is a > > standard part of the system but isn't part of mtree since it > > depends on > > local-system policy. It's also different from /home in that we > > *do* want > > it to be a standard place for updates, of course. I think there's > > really > > not a ton of precedent either way: we don't have any other mount > > points > > in there for file systems that may or may not exist depending on > > circumstances, as far as I can tell. My worry with having it in > > mtree is > > that having it exist but potentially be a directory rather than an > > actual ESP requires that update tools be a little smarter and errors > > will be a little less obvious, since updates that don't pay enough > > attention will be a bit more likely to splat files there assuming > > there > > is an ESP even if makes no sense. It's a weak consideration either > > way, > > I think. > > > > > > Yea. After a few hours of reflection, I've found that I could go > > either way and am having trouble understanding why I was so dead set > > this morning on a particular way. Chalk it up to me being a little > > extra grumpy at surprise changes. > > > > This one seems less like local policy than /home, but there's still a > > local aspect: Do I mount by default, and where. I think we should > > always, though, have a fstab entry as we'll need to update it from > > time to time. Even Windows has a nominal drive that it uses to mount > > the ESP, even if it isn't mounted by default. That's used to update it > > when scripts and such need to do that (or if you're the victim of an > > upgrade script that did too much that now needs to be undone). I think > > we should be similar in that regard. This would also let us take the > > automation of updates to the next level if we can rely on some basic > > things. > > That makes sense to me. There's also still the issue of non-EFI systems, > that differ only by install-time configuration from non-EFI systems. One > of my worries of having /boot/efi always exist is that a non-EFI system > may try to "update" the EFI by poking around in the empty /boot/efi and > think it has updated/installed something useful but has in reality done > nothing. But it's a tricky situation all around. I would think that during an update, which for me implies a system that is booted and running, that the definative answer to "are we an EFI system that needs to update EFI code" is infact machdep.bootmethod=EFI. Existance/absence of a directory, or an entry in /etc/fstab is a poor quality indicator. Is it even possible to create a dual mode installed FreeBSD system that boots in either BIOS or EFI mode? I know we do that for the installed media, but that is hybrid media, and I do not think that the bsdinstall can create such an installation on a disk. > -Nathan > > Warner > > > Warner -- Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4EB11ECA-1788-49D2-9362-09D955C8D695>