Would you like me to commit it?

LGTM

So in Linux icmpv6 logic the PTB message is processed in two steps:

Hmm, ok I see.
So if I understand correctly you propose to remove the PMTU handling in icmpv6, and parse it only in the protocol layer?
I suppose that it will reduce the attack surface.
Also the RFC9260 you linked says that we SHOULD, and not MUST process PMTUD.
So I suppose that in some cases it's ok to ignore this packet.

In D35871#814672, @tuexen wrote:

I think such a packet should never be used to change the host cache. For SCTP over UDP or TCP over UDP, we do the same validation as done for SCTP or TCP. In case an application is running on top of UDP, the application must validate that the reflected packet is OK before changing the host cache. Right now, this is not possible. But one could, for connected UDP socket, provide the encapsulated packet using MSG_NOTIFICATION, let the application do its verification and if that is successful allow a socket option to put the data into the host cache. Willing to work on that...

In D35871#814668, @rscheff wrote:

But when the protocol inside is TCP / SCTP, a validation would be possible. Especially when the hostcache of a protocol other than IP is being modified. The returned PTB ICMP message is supposed to hold as much of the offending packet's header as possible.

At least with stateful protocols, this could also dramatically reduce the attack surface of spoofed PTB error messages, not?

In other words in IPv4 PMTU information is used in transport layer, e.g. TCP uses it to lower MSS.
In IPv6 world the ip6_output needs to know PMTU in order to fragment the packet.

In D35871#814640, @tuexen wrote:

For IPv6, some input validation is done by the ICMP layer. However, then the TCP host cache is updated without any further validation by the transport layer and finally the processing is delegated to the transport layer. So an attacker does not have to guess the transport layer information to reduce the PMTU, since the ICMPv6 performs the action even if the transport layer detects the attack and does not perform the action (again).

So why is the modification of the TCP hostcache performed by the ICMPv6 stack at all? In my view, this just allows attacks...

In D35871#814577, @markj wrote:

Would it be difficult to add a regression test case for this using scapy and utilities in tests/sys/netinet6?

Committed in 89fb4802f9a64a1eba6475c3e467e614b69052a4

Fixed in D35759.

Use vm_map_fixed to map shared page when no address randomization is applied.

Use vm_map_fixed to allocate the guard page.

• Pass the shared page size in kinfo_vm_layout and use that information in kern_copyin test.
• Map the guard page at the top of UVA, instead of mapping it at the usual location of the shared page.

Pass the shared page address in kinfo_vm_layout struct and use it in kern_copyin test.

Check if the shared page address is randomized in kern_copyin test.

In D35349#803460, @kib wrote:

Why? As discussed earlier, if the shared page is randomized and the guard is mapped at the top of the UVA, map(MAP_FIXED) something at this location.

In D35349#803266, @kib wrote:

I still think that the guard at location of fixed shp is required.

• If the address randomization is applied map a guard page to the usual location of the shared page.
• Remove kern_copyin test. It doesn't work with the guard page at the top of user address space.

In D35392#803339, @emaste wrote:

@mindal_semihalf.com do you have these changes in a git repo somewhere? I can merge them as they're reviewed/approved.

• Move shared page ASLR flag from vmspace to image_params.

• Rename vm_sp_base to vm_shp_base
• Introduce the PROC_HAS_SHP macro
• Don't mention a.out in a comment.

Add an error message if mapping shared page fails

Copy offsets unconditionally in exec_sysvec_init_secondary (as suggested by kib in D35349).

In D35349#802348, @kib wrote:

For commit, I still believe that the split of the patch into two parts, one where you switch code to use base + offsets, and another where the actual randomization occurs, is the best. Could you, please, split the reviews?

Split the review into three parts.

I took a look at the linuxulator again and actually I don't think this patch will cause an issue there.
All ASLR related features, including this one are only activated if SV_ASLR flag is set in sysentvec.
None of the linuxulator sysentvecs have this flag set, so we should be fine here.
On a side note this means that the linuxulator binaries are not using ASLR at all.

• Replace PAGE_SIZE with sysconf(_SC_PAGESIZE).
• Store offsets to various shared page segments, instead of their base addresses.
• Store the shared page base address in struct vmspace, instead its offset to the default location.

In D35349#802006, @kib wrote:

Well, with some accuracy and care it is possible. At least, all variables should be static or hidden (and I do not think that non-local vars can exists in vdso at all, at least while the page is mapped rx). So the first step is to build with -fPIC and then mangle code enough to ensure that no relocations are produced. But this is still quite fragile and depends on the compiler use that it is not intended to.

My belief is that the only reliable option is to write it in asm, to have tight-controlled code.

In D35349#802004, @mindal_semihalf.com wrote:

In D35349#801992, @lattera-gmail.com wrote:

In D35349#801990, @kib wrote:

In D35349#801982, @mindal_semihalf.com wrote:

Sorry for the radio silence. I discovered that this patch, in its current form breaks Linuxulator VDSO clock routines.
Basically the problem is that the Linux VDSO glue code needs to read vdso_timekeep, that is stored in the shared page.
I have to figure out a fix for this first, before proceeding with this.
Once I have something I'll either open up a new phabricator revision, or update this one.

It is probably between too hard to impossible, due to linux vdso written in C. It requires PIC code, and it is almost always requires GOT and performing relocations before the code can work.

IMO it is enough to exclude linux ABI from shared page randomization.

That's what we noticed when HardenedBSD implemented VDSO randomization as well. The way the linuxulator's VDSO is implemented is not too friendly towards ASLR.

Ok, I'll go with that.
I guess that the best way to do this is to add an opt-in flag to sysentvec.

In D35349#801992, @lattera-gmail.com wrote:

In D35349#801990, @kib wrote:

In D35349#801982, @mindal_semihalf.com wrote:

Sorry for the radio silence. I discovered that this patch, in its current form breaks Linuxulator VDSO clock routines.
Basically the problem is that the Linux VDSO glue code needs to read vdso_timekeep, that is stored in the shared page.
I have to figure out a fix for this first, before proceeding with this.
Once I have something I'll either open up a new phabricator revision, or update this one.

It is probably between too hard to impossible, due to linux vdso written in C. It requires PIC code, and it is almost always requires GOT and performing relocations before the code can work.

IMO it is enough to exclude linux ABI from shared page randomization.

That's what we noticed when HardenedBSD implemented VDSO randomization as well. The way the linuxulator's VDSO is implemented is not too friendly towards ASLR.

Sorry for the radio silence. I discovered that this patch, in its current form breaks Linuxulator VDSO clock routines.
Basically the problem is that the Linux VDSO glue code needs to read vdso_timekeep, that is stored in the shared page.
I have to figure out a fix for this first, before proceeding with this.
Once I have something I'll either open up a new phabricator revision, or update this one.

Committed in ecdc04d006.

Cast both sides of comparison to size_t.

In D34907#792891, @kib wrote:

About the point that PMR only makes sense to disable globally, I am also not sure. For instance, if PMR was set up for something that can be externally plugged, it might be good idea to keep it enabled still.

In D34907#792838, @kib wrote:

As a second though, this control is global. Do you want it to be per-DMAR unit, in fact? [I am fine with this refine to be done later]

Hide the early disabling behind a hw.dmar.pmr.disable tunable. (Off by default.)
Leave the other places that call dmar_disable_protected_regions untouched.
Check the PMR state before disabling it. Without this a timeout is observed when we try to disable it the second time.

In D34256#786726, @mhorne wrote:

@mindal_semihalf.com are you happy with the changes?

Hmm, I just did a simple test by adding a call to kdb_backtrace to ofw_bus_default_get_node and removing ofw_pci.c from the build.
The device in backtrace is attached to a standard pci bus that doesn't know anything about ofw:

Yes, I mean we end up in kobj_error_method. And that's a very real scenario. In newbus, the default method is only applied for the given class and its subclasses. *Not for all classes*

This means that all controllers (not derived from ofwbus), pci or other buses not derived from ofw/simplebus are subject to this error.

This only applies to buses derived from simple/ofwbus, not to others (e.g. pci). For other device objects, the ofw_bus_get_node() function returns ENXIO (which may be a valid node ID). However, calling a class function on an object that is not derived from the given class should be considered an error.

In D33926#776749, @sjg wrote:

Actually, what does the kernel do with the manifest? or the hash of the manifest?

Once root is mounted, mac_veriexec verifies the manifest hash, and parses the manifest. Each file listed in the manifest gets resolved into a vnode and is added to mac_veriexec with its corresponding hash and flags ; the vnode can then later be verified when accessed. When the system is ready, mac_veriexec is already in "loaded active enforced" state. Without this mac_veriexec would not know of any file, so no program would be allowed to run, not even veriexec(8).

Currently mac_veriexec does not know anything about manifest parsing - that is the job of sbin/veriexec

I think it would be less disruption to the existing model for the loader to provide a hash of the manifest - that gets fed to mac_veriexec in the same manner that veriexec does.
Then set the state to loaded,active and then veriexec can use O_VERIFY to open that manifest and feed its content to mac_veriexec

OR you could just do all that via a kernel module loaded by loader, that effectively makes calls to initialize mac_veriexec - that would minimize the impact to existing usage, since anyone who doesn't want this need not load that module.

In D34031#776851, @mmel wrote:

I don't think that's right. We can't expect that all indirect descendants of simplebus were instantiated by simplebus. Typically all enumerable buses (e.g. pci or multifunction devices represented by single FDT node) are not derived from simplebus. You cannot use ofw_bus_get_node(child) on those.
IMHO we can have a single generic implementation of <foo>_get_property for a given bus , but it should be explicitly defined in the device_methods structure for all appropriate drives.

@bz ping

In D34027#775552, @mmel wrote:

Yes, I've applied all the patches. The problem is that the HS SD card (i.e. without debugging) also doesn't work and I don't know why.

In D34027#775532, @mmel wrote:

Due to problems with the D32706 I can't verify this on Honeycomb, plus I've never been able to have a working HW tuning. The current state is that this whole series of patches leaves Honeycomb with a non-functioning SD and eMMC. I currently have almost no free time to hack this problem, so everything is taking forever, sorry :(

In other words, HW tuning on the LX2160 is still not working, but this failure may be caused by any of previous patch from this series.

I think it might be better to keep the file sorted the way it was before this change.
This will make this patch much easier to read.

In D33457#774705, @bz wrote:

I keep meaning to give it a try for my local application; after all I think I triggered this? I cannot even remember. If you have a few more days I'll try hard; otherwise please don't wait for me.

@andrew Do you have any more thoughts on this?

Rebase on D34031