On Tue, Feb 25, 2025 at 04:12:40PM -0800, Jeff Xu wrote: > On Tue, Feb 25, 2025 at 7:18 AM Lorenzo Stoakes > <lorenzo.stoakes@xxxxxxxxxx> wrote: > > > > Jeff - looking further in this series, I asked for a couple things for this > > series which you've not provided: > > > > 1. Some assurance based on code that the kernel-side code doesn't rely on > > VDSO/VVAR etc. mapping. I gave up waiting for this and went and checked > > myself, it looks fine for arm64, x86-64. But it might have been nice had > > you done it :) Apologies if you had and I just missed it. > > > Thanks for checking this. > Do ppc in kernel code unmap/remap vdso ? > > I am aware that userspace can remap/unmap special mappings, but I > don't know if the kernel will remap/unmap a special mapping. Could you > please point out the code ? Again as discussed in other thread, let's leave this until as/when you try to attack PPC. I am not 100% this is the case, I may be mistaken sure, but gathered it _might_ be. > > > > 2. Tests - could you please add some tests to assert that mremap() fails > > for VDSO for instance? You've edited an existing test for VDSO in x86 to > > skip the test should this be enabled, but this is not the same as a self > > test. And obviously doesn't cover arm64. > > > > This should be relatively strightforward right? You already have code > > for finding out whether VDSO is msealed, so just use that to see if you > > skip, then attempt mremap(), mmap() over etc. + assert it fails. > > > > Ideally these tests would cover all the cases you've changed. > > > It is not as easy. > > The config is disabled by default. And I don't know a way to detect > KCONFIG from selftest itself. Without this, I can't reasonably > determine the test result. Please in future let's try to get this kind of response at the point of the request being made :) makes life much easier. So I think it is easy actually. As I say here (perhaps you missed it?) you literally already have code you added to the x86 test to prevent test failure. So you essentially look for [vdso] or whatever, then you look up to see if it is sealed, ensure an mremap() fails. Of course this doesn't check to see if it _should_ be enabled or not. I'm being nice by not _insisting_ we export a way for you to determine whether this config option is enabled or not for the sake of a test (since I don't want to hold up this series). But that'd be nice! Maybe in a /sys/kernel/security endpoint... ...but I think we can potentially add this later on so we don't hold things up here/add yet more to the series. I suspect you and Kees alike would prioritise getting _this series_ in at this point :) You could, if you wanted to, check to see if /proc/config.gz exists and zgrep it (only there if CONFIG_IKCONFIG_PROC is set) and assert based on that, but you know kinda hacky. But anyway, FWIW I think it'd be useful to assert that mremap() or munmap() fails here for system mappings for the sake of it. I guess this is, in effect, only checking mseal-ing works right? But it at least asserts the existence of the thing, and that it behaves. Later on, when you add some way of observing that it's enabled or not, you can extend the test to check this. Thanks!
