Le 20/07/2016 à 07:07, Takashi YAMAMOTO a écrit :
On Wed, Jul 20, 2016 at 3:54 AM, Maxime Villard <m...@netbsd.org <mailto:m...@netbsd.org>> wrote: Module Name: src Committed By: maxv Date: Tue Jul 19 18:54:45 UTC 2016 Modified Files: src/sys/arch/x86/x86: pmap.c Log Message: This loop makes no sense at all. - can you provide more meaningful commit message? - why don't you remove it then?
Look at the loop. It does not do anything. The bug was introduced six years ago in this commit [1], and there is clearly a mistake: the guy just added a loop around the code, but now the 'continue' refers to that new loop and not the underlying one. I could have fixed it to keep the original behavior, but in fact, I don't even see why this hack is relevant. This hack is supposed to make sure we never write-protect the PTE space, but as far as I can tell, the userland space is below it, and the kernel space is above it. So if the kernel or userland ever tries to write-protect this area, UVM will figure out it is not included in the space and will reject the request. In all cases, even if there were a way for someone to write-protect the PTE space, this hack would still be wrong: some pages in the range wouldn't be protected, and this is something the caller may not handle properly. And even beyond that, if write-protecting the PTE space were possible, it would be a huge flow in the design. So all this makes absolutely no sense at all. I didn't want to spend too much time on it, so I just added an XXX, for the next passer-by. I guess the best thing to do would be turning the 'continue' to a 'panic'; or just removing the loop completely, since the KASSERT below does mostly the same thing. [1] http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/x86/x86/pmap.c?only_with_tag=MAIN#rev1.112