On 16/08/25(Sat) 14:41, Mark Kettenis wrote: > > Date: Fri, 15 Aug 2025 15:53:25 +0200 > > From: Martin Pieuchot <m...@grenadille.net> > > > > On 15/08/25(Fri) 15:45, Mark Kettenis wrote: > > > > Date: Fri, 15 Aug 2025 14:06:06 +0200 > > > > From: Martin Pieuchot <m...@grenadille.net> > > > > > > > > On 15/08/25(Fri) 13:17, Mark Kettenis wrote: > > > > > > Date: Fri, 15 Aug 2025 11:51:18 +0200 > > > > > > From: Martin Pieuchot <m...@grenadille.net> > > > > > > > > > > > > On 15/08/25(Fri) 09:39, Miod Vallat wrote: > > > > > > > > Please don't. Keeping that page read-only is important for > > > > > > > > security. > > > > > > > > Maybe if nobody cares about the amd64 and i386 pmaps we should > > > > > > > > just > > > > > > > > delete those architectures? > > > > > > > > > > > > > > But remember, because the end argument was wrong (sz instead of > > > > > > > va + > > > > > > > sz), this call did *nothing*. > > > > > > > > > > > > > > At least the commented out code will be correct now. > > > > > > > > > > > > Exactly. Since you committed this code Mark it does nothing. > > > > > > That's > > > > > > what I said in the original report it's dead code. > > > > > > > > > > The original code (before I "broke" it) did an unmap of the memory. > > > > > BTW that means we need to fix the comment. > > > > > > > > Indeed. I'd be glad if you could do that when enable the > > > > uvm_map_protect() > > > > call. > > > > > > ok? > > > > Would you please integrate the proper refcount & error handling I fixed in > > my diff? > > I'm not sure the refcount and error handling in your diff makes sense. > I'm not sure the current refcount and error handling makes sense > either.
All I want is correct code. This code has already been copied and it took dv@ and I some iteration to get the refcount correct in vmm(4). The diff I produced might not be needed but is similar to what we have there and can be copied. > This sigobject is a permanent uvm object that is never going to be > freed. It gets created the first time we do an exec. And if for some > reason we fail to create it you won't be able to exec anything. Not > even init(8)! Sure, I'm fine with a panic. > So I think it makes more sense to simply panic if that uvm_map() call > fails. Not sure what to do with the uvm_map_protect() call. That > call should never fail. And even if it does fail, the kernel will > work just fine. So I'm inclined to leave it without an error check. Can we print a message in such case? I dislike silent failures. > At that point the reference counting should be really simple. The > uao_create() call already gives us a reference. And since we no > longer unmap the object we don't need another one. So we can actually > drop the uao_reference() call that we have right now. Right. > I would rather make those changes in a separate commit though. Go for it. > > Are we sure this won't explode on other architectures? Should that be > > tested? > > We tested a few other architectures while narrowing down the breakage > to amd64 and i386. And the pmap breakage was rather specific to those > architectures. So I don't expect any fallout. Then go for it.
