Re: [sctp fix] Re: CVS commit: src/sys/kern
Le 28/04/2020 à 09:16, Luke Mewburn a écrit : On 20-04-26 18:15, Maxime Villard wrote: | - There was no demonstrated use-case justifying importing it. In addition, |major OSes like Windows and macOS do not implement SCTP. There just is no |demand for SCTP on the market; and on NetBSD, proportionally even less. SCTP is used in mobile telco environments; the control plane for 3G and 4G networks uses SCTP (or TCP as an option, but mostly SCTP). That may be true. I am mostly aware of the multimedia use cases. Le 01/05/2020 à 18:46, m...@netbsd.org a écrit : We can setup an equivalence: put as much effort into the SCTP removal proposal as there was for the SCTP introduction proposal. Since SCTP was just dropped in src without any prior discussion, I don't think we need any discussion for removing it. That would be fair, yes. Le 02/05/2020 à 13:55, m...@netbsd.org a écrit : I'm sorry for picking on SCTP in particular. Apparently it was added because it was listed in src/doc/roadmaps.networking (and it's still listed there). But this doesn't address your own point, does it. The one-liner in src/doc/roadmaps/networking gives no explanation on why we would want SCTP in the first place. It doesn't even say where the code was imported from. This brings the question of who, exactly, made this list. Several of the items on this wanted list are actively *not* wanted, as Christos noted in five of his "Comment[christos]". At this point there is no doubt that SCTP in the NetBSD kernel has no future. The only viable option I see is usrsctp: https://github.com/sctplab/usrsctp A userland version of the code, but portable, and actively maintained. While here, notice the crazy buffer overflows that were fixed in it and are still present in the NetBSD SCTP kernel code... Adds to my point, that the code is of extremely poor quality. Maxime
[mii locking] Re: CVS commit: src/sys
Module Name:src Committed By: thorpej Date: Sun Mar 15 23:04:51 UTC 2020 Modified Files: src/sys/arch/arm/amlogic: gxlphy.c src/sys/arch/x86/pci: if_vmx.c src/sys/dev/mii: acphy.c amhphy.c atphy.c bmtphy.c brgphy.c ciphy.c dmphy.c etphy.c exphy.c gentbi.c glxtphy.c gphyter.c icsphy.c igphy.c ihphy.c ikphy.c inphy.c iophy.c ipgphy.c jmphy.c lxtphy.c makphy.c micphy.c mii.c mii_ethersubr.c mii_physubr.c miivar.h mvphy.c nsphy.c nsphyter.c pnaphy.c qsphy.c rdcphy.c rgephy.c rlphy.c smscphy.c sqphy.c tlphy.c tqphy.c ukphy.c ukphy_subr.c src/sys/dev/pci: if_mcx.c if_wm.c src/sys/dev/pci/ixgbe: ixgbe.c ixv.c src/sys/dev/usb: if_atu.c if_atureg.h if_aue.c if_axe.c if_axen.c if_cdce.c if_cue.c if_kue.c if_mos.c if_mue.c if_otus.c if_otusvar.h if_rum.c if_rumvar.h if_run.c if_runvar.h if_smsc.c if_udav.c if_upgt.c if_upgtvar.h if_upl.c if_ural.c if_uralvar.h if_ure.c if_url.c if_urndis.c if_urtw.c if_urtwn.c if_urtwnvar.h if_urtwreg.h if_zyd.c if_zydreg.h usbnet.c usbnet.h src/sys/net: if_media.c if_media.h src/sys/net80211: ieee80211.c ieee80211_netbsd.h ieee80211_var.h Log Message: Define and implement a locking protocol for the ifmedia / mii layers: - MP-safe drivers provide a mutex to ifmedia that is used to serialize access to media-related structures / hardware regsiters. Converted drivers use the new ifmedia_init_with_lock() function for this. The new name is provided to ease the transition. - Un-converted drivers continue to call ifmedia_init(), which will supply a compatibility lock to be used instead. Several media-related entry points must be aware of this compatibility lock, and are able to acquire it recursively a limited number of times, if needed. This is a SPIN mutex with priority IPL_NET. - This same lock is used to serialize access to PHY registers and other MII-related data structures. The PHY drivers are modified to acquire and release the lock, as needed, and assert the lock is held as a diagnostic aid. The "usbnet" framework has had an overhaul of its internal locking protocols to fit in with the media / mii changes, and the drivers adapted. USB wifi drivers have been changed to provide their own adaptive mutex to the ifmedia later via a new ieee80211_media_init_with_lock() function. This is required because the USB drivers need an adaptive mutex. Besised "usbnet", a few other drivers are converted: vmx, wm, ixgbe / ixv. mcx also now calls ifmedia_init_with_lock() because it needs to also use an adaptive mutex. The mcx driver still needs to be fully converted to NET_MPSAFE. Since this change if_udav.c doesn't work. Simply plugging a USB-to-ethernet device triggers a page fault on mutex_enter in udav_attach. Quickly looking at the code: 240 usbnet_lock_core(un); 241 usbnet_busy(un); 242 243 // /* reset the adapter */ 244 // udav_reset(un); 245 246 usbnet_attach(un, "udavdet"); usbnet_lock_core uses un_pri, but un_pri is initialized only in usbnet_attach.
Re: [statfs12] CVS commit: src
Le 27/06/2020 à 17:50, Christos Zoulas a écrit : Please revert all of this change. First, there was a clear vulnerability in this change, which I fixed in: https://mail-index.netbsd.org/source-changes/2020/06/27/msg118731.html Then, as I said in the change, there are additional problems: 137 static __inline int 138 statvfs_to_statfs12_copy(const void *vs, void *vs12, size_t l) 139 { 140 struct statfs12 *s12 = STATVFSBUF_GET(); 141 int error; 142 143 statvfs_to_statfs12(vs, s12); 144 error = copyout(s12, vs12, l); 145 STATVFSBUF_PUT(s12); 146 147 return error; 148 } STATVFSBUF_GET() allocates struct statvfs, but here we're using struct statfs12. How can this be expected to be correct? It is larger than needed, so it works. Why insist on using the wrong structure, when you could just as easily use the correct structure? I don't get the point. Maxime
[statfs12] Re: CVS commit: src
Module Name:src Committed By: christos Date: Fri Oct 4 01:28:03 UTC 2019 Modified Files: src/lib/libc/compat/sys: compat_statfs.c src/sys/compat/common: vfs_syscalls_20.c src/sys/compat/sys: mount.h Log Message: deduplicate the conversion function from statvfs -> statfs12 To generate a diff of this commit: cvs rdiff -u -r1.8 -r1.9 src/lib/libc/compat/sys/compat_statfs.c cvs rdiff -u -r1.43 -r1.44 src/sys/compat/common/vfs_syscalls_20.c cvs rdiff -u -r1.10 -r1.11 src/sys/compat/sys/mount.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Please revert all of this change. First, there was a clear vulnerability in this change, which I fixed in: https://mail-index.netbsd.org/source-changes/2020/06/27/msg118731.html Then, as I said in the change, there are additional problems: 137 static __inline int 138 statvfs_to_statfs12_copy(const void *vs, void *vs12, size_t l) 139 { 140 struct statfs12 *s12 = STATVFSBUF_GET(); 141 int error; 142 143 statvfs_to_statfs12(vs, s12); 144 error = copyout(s12, vs12, l); 145 STATVFSBUF_PUT(s12); 146 147 return error; 148 } STATVFSBUF_GET() allocates struct statvfs, but here we're using struct statfs12. How can this be expected to be correct? Then the copyout is done with a size, and again there are problems here. In compat_20_sys_getfsstat() the size given is struct statvfs90, which matches neither the filled size nor the allocated size. The other callers have even bigger problems. For example compat_20_sys_statfs() passes zero as size. So the result simply never gets copied out. How can this be expected to be correct? As far as I can tell the syscall simply cannot work now. Finally, I don't even understand what this change dedups. It just moved the functions from one place to another, introduced bugs in them, but didn't reduce the code size in any way. As I said, please revert all of this change, it is just plain wrong and hasn't received any testing. Thank you, Maxime
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
Le 03/06/2020 à 02:03, Kamil Rytarowski a écrit : On 03.06.2020 01:49, Andrew Doran wrote: On the assembly thing recall that recently you expressed a desire to remove all of the amd64 assembly string functions from libc because of sanitizers - I invested my time to do up a little demo to try and show you why that's not a good idea: http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html Please note that interceptors for string functions are not just some extra burden, but also very useful approach to feedback a fuzzer through additional coverage. At least libFuzzer and honggfuzz wrap many kinds of string functions and use it for fuzzing. We should add a special mode in KCOV to feedback userland (syzkaller) with traces from string functions. https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 Yes, and not just that either. When you use ASM instead of C, you basically prevent _any kind_ of useful transformation the compiler could make. It includes sanitizers, but also coverage as you said; and also retpoline, PAC, BTI, CET, SafeStack, and in short, a very big bunch of modern features. Favoring C rather than ASM in the general sense offers much bigger benefits than just "it accomodates kMSan". Maxime
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
Le 03/06/2020 à 01:49, Andrew Doran a écrit : On Tue, Jun 02, 2020 at 08:41:53AM +0200, Maxime Villard wrote: Le 02/06/2020 ? 00:58, Andrew Doran a ?crit?: Module Name:src Committed By: ad Date: Mon Jun 1 22:58:06 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/amd64/include: frameasm.h Log Message: Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com Instrument STOS/MOVS for KMSAN to unbreak it. To generate a diff of this commit: cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Can you just stop ignoring the remarks that are made? That's up to you Maxime. If you habitually make it difficult for people to come to a reasonable compromise with you, then you're asking to not be taken seriously and will find yourself being ignored. You are confused. I asked for KMSAN to be unbroken, and proposed an alternative, which is atomic_load_relaxed. You were free to explain why my point was a bad idea or why it didn't matter, but you refused, and just added a hack on top of another. I'm afraid that's up to you. But whatever, it doesn't matter. Thanks for reverting the pmap.c changes, it is more correct now than before. I said explicitly that adding manual instrumentation here is _wrong_. I don't share your assessment in the general sense. It should be possible to instrument assembly code because KMSAN is useful AND we can't get by without assembly - there are some things that C just can't do (or do well enough). On the assembly thing recall that recently you expressed a desire to remove all of the amd64 assembly string functions from libc because of sanitizers - I invested my time to do up a little demo to try and show you why that's not a good idea: http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html I saw, yes. I answered explaining that a conversation with Ryo Shimizu had changed my mind a bit, and seeing your results (which as far as I can tell do not indicate a performance improvement significant enough to not be considered as noise), I asked you whether it was that relevant. You didn't follow up though. The system is a balancing act. There are lots of factors to be taken into account: maintainability, tooling like KMSAN, user's varying needs, the capabilites of different machines, performance, feature set and so on, and dare I say it even the enjoyment of the people working on the project is important too. Myopic focus on one factor only to the detriment of others is no good. I am well aware of that. The kMSan ASM fixups are limited to args/returns, and that is part of a sensical policy that _should not_ be changed without a good reason. x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions you made to them in pmap.c, not one is justified. memcpy/memset were all correct. I introduced these functions as a compromise because you were unhappy with use of memcpy() to copy PDEs. See: http://mail-index.netbsd.org/port-amd64/2020/05/23/msg003280.html You wrote: In the [XXX] window, the PTEs could be used by userland. If you copied them using memcpy(), some parts of the bytes could contain stale values. Sure, I was explicitly referring to SVS, which has an unusual way of accessing PTEs (contrary to pmap), which is why it needs special atomic care that other places do not. Live PDEs are also copied in pmap.c so I made a change there too. After that I decided "why not" and used the new functions everywhere PDEs/PTEs or pages are block zeroed / copied. But I'm also happy with memcpy()/memset(). Either way will work. In fairness I do work too fast sometimes. The only reason you made these big unneeded changes is for SVS not to take the bus lock, There is no bus lock on x86 (not since the 1990s anyway). but as was said already, atomic_load_relaxed will do what you want without the need for these functions. Please revert _both changes now_, this one and the previous one which introduced both functions, and let's use atomic_load_relaxed. You're focusing on only one factor. I'll explain in detail. Here is the original code I replaced: 685 static inline pt_entry_t 686 svs_pte_atomic_read(struct pmap *pmap, size_t idx) 687 { 688/* 689 * XXX: We don't have a basic atomic_fetch_64 function? 690 */ 691return atomic_cas_64(>pm_pdir[idx], 666, 666); 692 } ... 717/* User slots. */ 718for (i = 0; i < PDIR_SLOT_USERLIM; i++) { 719pte = svs_pte_atomic_read(pmap, i); 720ci->ci_svs_updir[i] = pte; 721} There's no need for an atomic op there because fetches on x86 are b
[stos, again] Re: CVS commit: src/sys/arch/amd64
Le 02/06/2020 à 00:58, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Mon Jun 1 22:58:06 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/amd64/include: frameasm.h Log Message: Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com Instrument STOS/MOVS for KMSAN to unbreak it. To generate a diff of this commit: cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Can you just stop ignoring the remarks that are made? I said explicitly that adding manual instrumentation here is _wrong_. The kMSan ASM fixups are limited to args/returns, and that is part of a sensical policy that _should not_ be changed without a good reason. x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions you made to them in pmap.c, not one is justified. memcpy/memset were all correct. The only reason you made these big unneeded changes is for SVS not to take the bus lock, but as was said already, atomic_load_relaxed will do what you want without the need for these functions. Please revert _both changes now_, this one and the previous one which introduced both functions, and let's use atomic_load_relaxed. Maxime
Re: [virtio] Re: CVS commit: src/sys/dev/pci
Le 01/06/2020 à 03:23, Shoichi Yamaguchi a écrit : Hi, On Wed, May 27, 2020 at 8:47 PM Shoichi Yamaguchi wrote: I modified virtio(4) not to allocate unused memory. I guess it fixes the issue. Could you check this? I confirmed your closing the report on syzbot. https://syzkaller.appspot.com/bug?id=73f690e8a3d8e304407808555f1dacfa004685d1 Thank you for your response. Sorry for my lack of response -- I was waiting for the kMSan instance to sync, but it is currently down, and has been down for four days and a half now. The kMSan instance got the time to run 24h before it broke for unrelated reasons. 24h before your patch, it triggered the bug 6 times. 24h after your patch, it triggered the bug 0 times. So indeed, we can call it fixed, thanks for the fix
Re: [stos] Re: CVS commit: src/sys/arch
Le 28/05/2020 à 23:58, Andrew Doran a écrit : On Thu, May 28, 2020 at 07:06:04PM +0200, Maxime Villard wrote: Le 27/05/2020 ? 21:58, Maxime Villard a ?crit?: Le 27/05/2020 ? 21:33, Andrew Doran a ?crit?: Module Name:??? src Committed By:??? ad Date:??? Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero ?? and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Very good. I see a function in subr_msan.c that looks like it does the needful here: void __msan_instrument_asm_store(void *addr, size_t size) I don't see any uses in tree. Is there a reason for that? The functions that begin by __msan_* are called from the compiler-generated instrumentation directly, not from the kernel (even though they could). Adding calls to this function from asm would be a big hack that I don't want in the kernel, and it wouldn't be addressing the real problem, which is, that the introduction of x86_movs/x86_stos is unnecessary in the first place, and the way they are used right now is just wrong -- memcpy/memset should have stayed in place. The whole change you made is for svs_pdir_switch() to use quads, but why not use atomic_load_relaxed? With that it should fetch quads without taking the bus lock, and we don't have to resort to ugly ASM functions. Thanks, Maxime
Re: [stos] Re: CVS commit: src/sys/arch
Le 28/05/2020 à 19:06, Maxime Villard a écrit : Le 27/05/2020 à 21:58, Maxime Villard a écrit : Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name: src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Thanks, Maxime "MI" -> "MD"
Re: [stos] Re: CVS commit: src/sys/arch
Le 27/05/2020 à 21:58, Maxime Villard a écrit : Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name: src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Thanks, Maxime
[stos] Re: CVS commit: src/sys/arch
Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though
[virtio] Re: CVS commit: src/sys/dev/pci
Hi, I don't know if this is related to your changes, but kMSan detected one uninit variable in virtio 3h ago: https://syzkaller.appspot.com/text?tag=CrashReport=12084ef610 [ 153.4370851] panic: MSan: Uninitialized Kmem Memory From virtio_pci_setup_interrupts() [ 153.4448669] cpu0: Begin traceback... [ 153.4448669] vpanic() at netbsd:vpanic+0x7c1 sys/kern/subr_prf.c:288 [ 153.4632004] panic() at netbsd:panic+0x1ad sys/kern/subr_prf.c:209 [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 kmsan_report_inline sys/kern/subr_msan.c:239 [inline] [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 sys/kern/subr_msan.c:612 [ 153.4931985] virtio_pci_free_interrupts() at netbsd:virtio_pci_free_interrupts+0x1b4 sys/dev/pci/virtio_pci.c:740 [ 153.5132006] virtio_child_detach() at netbsd:virtio_child_detach+0x116 sys/dev/pci/virtio.c:924 [ 153.5331982] vioscsi_detach() at netbsd:vioscsi_detach+0x40d sys/dev/pci/vioscsi.c:244 [ 153.5532009] config_detach() at netbsd:config_detach+0x7e3 sys/kern/subr_autoconf.c:1760 [ 153.5732017] config_detach_all() at netbsd:config_detach_all+0x29a sys/kern/subr_autoconf.c:1906 [ 153.5831984] cpu_reboot() at netbsd:cpu_reboot+0x290 sys/arch/amd64/amd64/machdep.c:700 [ 153.6031986] kern_reboot() at netbsd:kern_reboot+0x18f sys/kern/kern_reboot.c:73 [ 153.6231980] sys_reboot() at netbsd:sys_reboot+0x28d This means that some memory allocated by virtio_pci_setup_interrupts() on the kmem allocator was not initialized, and later one access to it was made by virtio_pci_free_interrupts() at l.740 of the file. Can you have a look? Thanks, Maxime
[acl] Re: CVS commit: src
Module Name:src Committed By: christos Date: Sat May 16 18:31:54 UTC 2020 Modified Files: [...] Log Message: Add ACL support for FFS. From FreeBSD. This broke compilation on LLVM. https://syzkaller.appspot.com/text?tag=CrashReport=153178f610 Please fix
[cpufunc] Re: CVS commit: src/sys/arch
Module Name:src Committed By: ad Date: Tue May 19 21:40:55 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/i386/i386: cpufunc.S i386func.S Log Message: Make cpu_counter(), cpu_counter32() and tsc_get_timecount() into a single preemption-safe routine. To generate a diff of this commit: cvs rdiff -u -r1.52 -r1.53 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.40 -r1.41 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.21 -r1.22 src/sys/arch/i386/i386/i386func.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Aliasing functions with different prototypes is wrong. cpu_counter() returns a uint64_t, so it should have KMSAN_INIT_RET(8). The two other functions you aliased return uint32_t, so they should have KMSAN_INIT_RET(4). This can't be reconciled because of the alias. Please revert this change.
[sctp fix] Re: CVS commit: src/sys/kern
Le 26/04/2020 à 16:21, Jonathan A. Kollasch a écrit : > Module Name: src > Committed By: jakllsch > Date: Sun Apr 26 14:21:14 UTC 2020 > > Modified Files: > src/sys/kern: uipc_socket.c > > Log Message: > Implement SCTP bug fixes found by maxv@. > > Adding these seems to improve the SCTP situation. Yeah, thanks... I remember I had sent an email about these bugs, no one cared, so I just put big XXXs and left the broken thing as-is. That was more than a year ago. I remember also pointing out at some point the severe deficiencies of the SCTP code we have, with no one caring once again (not even me actually). In its current state (that is, the state it has been in ever since it was imported five years ago), our SCTP code is a near-perfect example of gigantic, buggy and useless bloat. Specifically, as far as I remember: - Last I checked, SCTP occupies, in number of lines of code, half of our IPv4 network stack. In other words, when it was imported, our kernel netinet stack suddenly doubled in size. I don't see how we will ever MP-ify all of that. - There was no demonstrated use-case justifying importing it. In addition, major OSes like Windows and macOS do not implement SCTP. There just is no demand for SCTP on the market; and on NetBSD, proportionally even less. - The code is of remarkably poor quality, with bugs in all directions, complex pieces of logic that seem rarely justified, and implementation errors like the pointer bugs you just fixed. The bloat and bugs are already evident as early as in the first twenty lines of code of the sctp_input() entry point. - IIRC the mbuf API usage is very poor (though I don't remember the specifics here; I must have kept paper notes somewhere). - It seems that no one is maintaining it? Nothing has improved over the last five years, and there are no apparent signs that this situation will ever change. There are piecemeal changes like yours and mine, to accommodate API changes and fix obvious bugs, and that's about it, as far as I can tell. At one point I hesitated about doing a big cleanup of SCTP. But I concluded that it is too structurally broken, and that fixing it is a waste of time; rewriting it would be faster and would result in a better, more functional, and easier to maintain code. Overall I think this is an example of bad policy. It's good to have support for new protocols, but at some point, we need to question whether landing 40K lines of buggy yet critical kernel code out of nowhere with no one maintaining it and little justification for the feature is a good thing to do. CC'ing core@ in case they are interested in making a useful statement for once. In all cases, this code is disabled by default, which is a good thing given its state, but results in us not giving a damn about it. Maxime
Re: CVS commit: src
Le 26/04/2020 à 14:05, Paul Goyette a écrit : > Why are these files being placed in src/tests/modules? Because I modeled my tests after the ufetchstore and threadpool tests which are both in this directory and provide user access to kernel internals via modules, which is exactly what I'm doing. Also it looks like the t_kcov one doesn't match your definition of what this directory should be about. > That directory is supposed to contain tests of the module feature; > it is not intended to place modules-that-support-tests-of-other- > features. > > Can you please put these in src/tests/sys/ or somewhere more > appropriate? > > > > > On Sun, 26 Apr 2020, Maxime Villard wrote: > >> Module Name: src >> Committed By: maxv >> Date: Sun Apr 26 09:08:41 UTC 2020 >> >> Modified Files: >> src/distrib/sets/lists/debug: md.amd64 >> src/distrib/sets/lists/tests: md.amd64 >> src/tests/modules: Makefile >> Added Files: >> src/tests/modules: t_x86_pte.c >> src/tests/modules/x86_pte_tester: Makefile x86_pte_tester.c >> >> Log Message: >> Add tests on the x86 PTEs. We scan the MMU page tables directly and verify >> certain properties. >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.107 -r1.108 src/distrib/sets/lists/debug/md.amd64 >> cvs rdiff -u -r1.8 -r1.9 src/distrib/sets/lists/tests/md.amd64 >> cvs rdiff -u -r1.18 -r1.19 src/tests/modules/Makefile >> cvs rdiff -u -r0 -r1.1 src/tests/modules/t_x86_pte.c >> cvs rdiff -u -r0 -r1.1 src/tests/modules/x86_pte_tester/Makefile \ >> src/tests/modules/x86_pte_tester/x86_pte_tester.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> >> >> !DSPAM:5ea54fda15308349521! >> >> > > ++--+---+ > | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | > | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | > | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | > ++--+---+
Re: CVS commit: src/sys/modules/compat_netbsd32
I almost got a heart attack between your first email and your second one, wondering how this code got re-enabled. Thanks for clarifying. Relevant example, by the way. Committed on August 20th 2019 at 09:32 https://mail-index.netbsd.org/source-changes/2019/08/20/msg108321.html Disabled by me the same day at 12:25 https://mail-index.netbsd.org/source-changes/2019/08/20/msg108332.html No rocket science here, I just saw that the code qualified as close to structurally deficient, and went ahead. In case you people are wondering, I think I did it with strictly no discussion. Unsurprisingly, it looks like it has saved us a lot of trouble here. This is what sane policy looks like. Le 19/04/2020 à 19:58, m...@netbsd.org a écrit : > Good news everyone. Does not affect any release at all. > Also might not have been a security issue, because christos did a weird > thing where it is compiled but somehow still disabled. > > On Sun, Apr 19, 2020 at 05:40:50PM +, Maya Rashish wrote: >> Module Name: src >> Committed By:maya >> Date:Sun Apr 19 17:40:50 UTC 2020 >> >> Modified Files: >> src/sys/modules/compat_netbsd32: Makefile >> >> Log Message: >> Turn off compat drm. >> XXX issue security advisory >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.32 -r1.33 src/sys/modules/compat_netbsd32/Makefile >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> > >> Modified files: >> >> Index: src/sys/modules/compat_netbsd32/Makefile >> diff -u src/sys/modules/compat_netbsd32/Makefile:1.32 >> src/sys/modules/compat_netbsd32/Makefile:1.33 >> --- src/sys/modules/compat_netbsd32/Makefile:1.32Thu Mar 12 15:02:29 2020 >> +++ src/sys/modules/compat_netbsd32/Makefile Sun Apr 19 17:40:49 2020 >> @@ -1,13 +1,13 @@ >> -# $NetBSD: Makefile,v 1.32 2020/03/12 15:02:29 pgoyette Exp $ >> +# $NetBSD: Makefile,v 1.33 2020/04/19 17:40:49 maya Exp $ >> >> .include "../Makefile.inc" >> .include "../Makefile.assym" >> >> KMOD= compat_netbsd32 >> >> -.if ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "x86_64" >> -NETBSD32_DRMKMS?=yes >> -.endif >> +#.if ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "x86_64" >> +#NETBSD32_DRMKMS?=yes >> +#.endif >> >> CPPFLAGS+= -DCOMPAT_NETBSD32 >> CPPFLAGS+= -DEXEC_ELF32 -DEXEC_ELF64
[disk changes] CVS commit: src/sys/dev/dkwedge
> Module Name:src > Committed By: jdolecek > Date: Sat Apr 11 16:00:34 UTC 2020 > > Modified Files: > src/sys/dev/dkwedge: dkwedge_apple.c dkwedge_bsdlabel.c dkwedge_gpt.c > dkwedge_mbr.c dkwedge_rdb.c It appears that since your recent changes, there is a systematic use-after-free: panic: ASan: Unauthorized Access in 0x...: Addr 0x... [2 bytes, read, PoolUseAfterFree] wdc_ata_bio() wdstart1() wd_diskstart() dk_start() bdev_strategy() spec_strategy() VOP_STRATEGY() genfs_getpages() VOP_GETPAGES() ubc_fault() uvm_fault_internal() trap() --- trap (number 6) --- copyout() uiomove() ubc_uiomove() ffs_read() VOP_READ() vn_read() dofileread() sys_read() syscall() This is reliably reproductible by just booting KASAN on amd64. Can you give a look? Thanks, Maxime
[vfs_cache] Re: CVS commit: src/sys/kern
Le 23/03/2020 à 21:02, Andrew Doran a écrit : > Module Name: src > Committed By: ad > Date: Mon Mar 23 20:02:14 UTC 2020 > > Modified Files: > src/sys/kern: vfs_cache.c > > Log Message: > cache_remove(): remove from the vnode list first, so cache_revlookup() > doesn't try to re-activate an entry no longer on the LRU list. > > > To generate a diff of this commit: > cvs rdiff -u -r1.133 -r1.134 src/sys/kern/vfs_cache.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. It appears that your recent changes in vfs_cache.c have introduced a use-after-free. Booting KASAN gives: ASan: Unauthorized Access In 0x...: Addr 0x... [1 byte, read, PoolUseAfterFree] It seems that the problem is here: 557 if (nameiop == CREATE && (cnflags & ISLASTCN) != 0) { 558 /* 559 * Last component and we are preparing to create 560 * the named object, so flush the negative cache 561 * entry. 562 */ 563 COUNT(ncs_badhits); 564 cache_remove(ncp, true);< HERE 565 hit = false; 566 } else { 567 COUNT(ncs_neghits); 568 SDT_PROBE(vfs, namecache, lookup, hit, dvp, name, 569 namelen, 0, 0); 570 /* found neg entry; vn is already null from above */ 571 hit = true; 572 } 573 if (iswht_ret != NULL) { 574 /* 575 * Restore the ISWHITEOUT flag saved earlier. 576 */ 577 *iswht_ret = ncp->nc_whiteout; <-- ouch 578 } else { 579 KASSERT(!ncp->nc_whiteout); <-- ouch 580 } cache_remove() frees 'ncp', and then 'ncp->nc_whiteout' is read. Maxime
Re: CVS commit: src
Le 26/03/2020 à 15:32, Jonathan A. Kollasch a écrit : > On Tue, Aug 14, 2018 at 02:49:14PM +0000, Maxime Villard wrote: >> Module Name: src >> Committed By:maxv >> Date:Tue Aug 14 14:49:14 UTC 2018 >> Log Message: >> Retire EtherIP, we have L2TP instead. > > Why was this not discussed? It was discussed. https://mail-index.netbsd.org/tech-net/2018/08/07/msg006998.html A bit of a blurted-out email using the bad word "sh*tty", which was ill-placed, but other than that I was not technically incorrect. > L2TP does not implement EtherIP > functionality, and is therefore NOT a replacement. Not sure what you mean? We have L2TPv3, and it does come as a replacement of EtherIP. Maxime
Re: CVS commit: src/sys/dev/usb
Le 23/03/2020 à 04:07, Roy Marples a écrit : > On 22/03/2020 08:30, Maxime Villard wrote: >> Overall "From OpenBSD" is a redflag for buggy and vulnerable code.. > > We should be above this, no software is perfect, not even ours. > > Roy You seem to be confusing one-off defects and structural deficiencies. That a plane crashes because of one slightly malformed screw, is a one-off defect. Yes, sh*t happens, that's statistical, and in the order of things. That a plane crashes because pilots have trained on a faulty simulator, are faced with incomplete emergency manuals, that don't document the faulty flight computer about to bring the plane down, itself installed to work around the plane's faulty airframe, is a big redflag for structural deficiencies. In that you could as well fix the simulator, fix the manuals, fix the computer, fix the airframe, that there would still be a consistent way for the plane to crash, because it is just so structurally deficient, that no one could honestly put any kind of trust in it. Damn, I love this analogy. Anyway, to come back to the point, I have come to notice that several organizations (very big ones sometimes...) produce code that is very close to structurally deficient, and that's a source of concern for our QA when that code gets imported. In the case of OpenBSD I don't know if it is recent or if it has always been like this, I would tend to think the latter. So yeah big redflag when I see a "from ...", that's an indication that the area needs attention. In all cases, these specific issues with if_umb are not urgent, because the driver is disabled by default in NetBSD. Interesting technical challenge though, if someone is interested! Maxime
Re: CVS commit: src/sys/dev/usb
Le 19/03/2020 à 08:49, Pierre Pronchery a écrit : > Module Name: src > Committed By: khorben > Date: Thu Mar 19 07:49:29 UTC 2020 > > Modified Files: > src/sys/dev/usb: if_umb.c > > Log Message: > When there is no network around the state timeout fires over and over again. > Change the printf into a log and only under IFF_DEBUG to reduce dmesg spam. > Loudly requested by beck@ OK deraadt@ FWIW, there is a number of potentially exploitable bugs in this driver, and they have been in my todo list for three months. Eg, follow umb_decode_response(), there are integer overflows that can trigger actual buffer overflows. Would you be interested in fixing the vulns? > From OpenBSD. Overall "From OpenBSD" is a redflag for buggy and vulnerable code.. Maxime
Re: CVS commit: src/sys/ufs/ufs
Le 27/02/2020 à 01:36, Simon Burge a écrit : > "Maxime Villard" wrote: > >> Module Name: src >> Committed By:maxv >> Date:Wed Feb 26 18:00:12 UTC 2020 >> >> Modified Files: >> >> src/sys/ufs/ufs: ufs_vnops.c >> >> Log Message: >> >> Zero out the padding in 'd_namlen', to prevent info leaks. Same logic as >> ufs_makedirentry(). > > Is it cleaner to just call pool_cache_get() with PR_ZERO? > > Cheers, > Simon. In this specific case there is already a clean macro that gives the number of padding bytes, so using it is cleaner/faster than zeroing the whole buffer. Maxime
Re: CVS commit: src/sys/kern
Le 08/03/2020 à 21:41, Andrew Doran a écrit : > On Sun, Mar 08, 2020 at 08:34:29AM +0100, Maxime Villard wrote: >> Le 08/03/2020 ? 01:31, Andrew Doran a ?crit?: >>> Module Name:src >>> Committed By: ad >>> Date: Sun Mar 8 00:31:19 UTC 2020 >>> >>> Modified Files: >>> src/sys/kern: subr_kmem.c >>> >>> Log Message: >>> KMEM_SIZE: append the size_t to the allocated buffer, rather than >>> prepending, so it doesn't screw up the alignment of the buffer. >>> >>> Reported-by: syzbot+c024c50570cccac51...@syzkaller.appspotmail.com >>> >>> >>> To generate a diff of this commit: >>> cvs rdiff -u -r1.78 -r1.79 src/sys/kern/subr_kmem.c >>> >>> Please note that diffs are not public domain; they are subject to the >>> copyright notices on the relevant files. >> >> This is wrong. The point of KMEM_SIZE is to store at a reliable location >> the size of the buffer, in order to then verify that the 'size' argument >> given to kmem_free() is correct. >> >> Here, you are using that size argument to compute the location, which >> breaks the whole point of the check. > > Sure I understand the frustration, but it's still correct according to > the parameters I set for it 10+ years ago, which were for it to be a > quick-n-dirty diagnostic aid. No, it is not. KMEM_SIZE has found real bugs. Now this useful feature has been lessened just to shut a sanitizer which was pointing another issue. Again, as said previously, the real bug here is that you are making a caller assume specific alignment from an allocator that _does not_ guarantee this alignment. To fix that, either (1) revert the assumption or (2) make it part of the allocator's contract to guarantee this alignment. I don't have a preference on (1) or (2), but the current design is more a hack than anything else, and the subsequent change in KMEM_SIZE is definitely wrong. >> Also it probably collides with the KASAN shadow. > > Hmm, is that purely an alignment issue then, because it works in 8 byte > cells? Otherwise it sounds like this should not be enabled with KASAN at > all. Not sure what you mean, but KASAN definitely needs to be enabled on kmem. I've checked the code, and actually it is fine regarding KASAN for now, because the computation of the redzone doesn't take KMEM_SIZE into account (and so isn't affected by the fact that the location changed).
Re: CVS commit: src
Le 08/03/2020 à 02:33, Andrew Doran a écrit : > On Sat, Mar 07, 2020 at 12:24:21PM +0100, Maxime Villard wrote: > >> Can we revert the "__aligned(COHERENCY_UNIT)" for now? There is no particular >> hurry to fix this bug, however the KUBSAN instance has been down for more >> than >> two months because of this, and it needs to be addressed. > > That should be quelled now. The change is not correct, see my answer in response to the commit. >> Similarly, the KASAN instance is currently crashing hard on: >> >> https://syzkaller.appspot.com/bug?id=1aa3f789d356bf04644bcef632bf8c2373398ba2 >> Dozens of thousands of times each day. This has been the case for two weeks, >> and it too needs to be addressed. > > That's been there since I started looking last year. Not sure what you mean? The history log indicates that it started on Jan 7th 2020. > I guess it's a false positive because the sanitiser probably thinks objects > are gone once pool_cache_put() is called, but the actual point of disposal > is the pool_cache dtor. There is no false positive because of that. If a dtor is there, the data is left as valid: 3124 #ifdef KASAN 3125/* If there is a ctor/dtor, leave the data as valid. */ 3126if (__predict_false(pc_has_ctor(pc) || pc_has_dtor(pc))) { 3127return; 3128} 3129 #endif And in all cases, the instance is compiled with POOL_QUARANTINE, which "cancels" caches, that is, pool_cache_put directly returns the object to the pool layer. So the buffer remains KASAN-valid, goes through dtor, then is made KASAN-invalid, and finally lands in the pool. Looking at the report, it looks like the use-after-free is on this line in mutex_oncpu(): 421 l = (lwp_t *)MUTEX_OWNER(owner); 422 ci = l->l_cpu; So mutex_oncpu() is called on a lock somehow (previously?) held by an LWP that now has been freed. If you look at the different reports, this issue is triggered in random places. Maybe one of your recent changes leaves the mutex as owned somehow? Thanks
Re: CVS commit: src/sys/kern
Le 08/03/2020 à 01:31, Andrew Doran a écrit : > Module Name: src > Committed By: ad > Date: Sun Mar 8 00:31:19 UTC 2020 > > Modified Files: > src/sys/kern: subr_kmem.c > > Log Message: > KMEM_SIZE: append the size_t to the allocated buffer, rather than > prepending, so it doesn't screw up the alignment of the buffer. > > Reported-by: syzbot+c024c50570cccac51...@syzkaller.appspotmail.com > > > To generate a diff of this commit: > cvs rdiff -u -r1.78 -r1.79 src/sys/kern/subr_kmem.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. This is wrong. The point of KMEM_SIZE is to store at a reliable location the size of the buffer, in order to then verify that the 'size' argument given to kmem_free() is correct. Here, you are using that size argument to compute the location, which breaks the whole point of the check. Also it probably collides with the KASAN shadow. Please revert this change. As said previously, if cacheline alignment is expected this way, then it should clearly be part of the kmem contract, and documented to be so. Thanks
Re: CVS commit: src
Le 25/02/2020 à 19:18, Maxime Villard a écrit : > Le 23/02/2020 à 23:19, Andrew Doran a écrit : >> On Fri, Feb 21, 2020 at 02:14:31PM +0100, Kamil Rytarowski wrote: >> >>> On 22.12.2019 20:47, Andrew Doran wrote: >>>> Module Name: src >>>> Committed By: ad >>>> Date: Sun Dec 22 19:47:35 UTC 2019 >>>> >>>> Modified Files: >>>> src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_ctldir.c >>>> src/sys/kern: vfs_mount.c vfs_subr.c vfs_syscalls.c >>>> src/sys/miscfs/genfs: genfs_vfsops.c >>>> src/sys/nfs: nfs_export.c >>>> src/sys/sys: mount.h vnode.h vnode_impl.h >>>> src/sys/ufs/lfs: ulfs_vfsops.c >>>> src/sys/ufs/ufs: ufs_vfsops.c ufs_wapbl.c >>>> >>>> Log Message: >>>> Make mntvnode_lock per-mount, and address false sharing of struct mount. >>>> >>> >>> This change broke kUBSan syzbot. >>> >>> The sanitizer is now very noisy as struct mount requires 64 byte alignment. >>> >>> http://netbsd.org/~kamil/kubsan/mount-alignment.txt >> >> I had a look this weekend. This is down to KMEM_SIZE messing up the >> alignment, so is a DIAGNOSTIC thing. The align_offset parameter to >> pool_cache() would be a nice easy way to solve this but it seems someone >> killed that off, so I'll need to give this some more thought. >> >> Andrew > > kmem guarantees alignment on 8 bytes, but not more. Changing the backend > allocators to enforce more alignment still may not result in aligned buffers, > because kmem has the right to modify the buffers for debugging purposes, like > with KMEM_SIZE. > > If you want a buffer aligned to a specific value, don't use kmem, rather a > pool(_cache) with COHERENCY_UNIT in "align". > > If the goal is to have kmem really enforce COHERENCY_UNIT alignment, then this > should be documented and the debugging features should be adapted to respect > that constraint. > > "align_offset" got removed because it increased complexity in subr_pool for > no reason (two users in all of the kernel, one was actually a bug). Can we revert the "__aligned(COHERENCY_UNIT)" for now? There is no particular hurry to fix this bug, however the KUBSAN instance has been down for more than two months because of this, and it needs to be addressed. Similarly, the KASAN instance is currently crashing hard on: https://syzkaller.appspot.com/bug?id=1aa3f789d356bf04644bcef632bf8c2373398ba2 Dozens of thousands of times each day. This has been the case for two weeks, and it too needs to be addressed. Thanks
Re: CVS commit: src
Le 23/02/2020 à 23:19, Andrew Doran a écrit : On Fri, Feb 21, 2020 at 02:14:31PM +0100, Kamil Rytarowski wrote: On 22.12.2019 20:47, Andrew Doran wrote: Module Name:src Committed By: ad Date: Sun Dec 22 19:47:35 UTC 2019 Modified Files: src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_ctldir.c src/sys/kern: vfs_mount.c vfs_subr.c vfs_syscalls.c src/sys/miscfs/genfs: genfs_vfsops.c src/sys/nfs: nfs_export.c src/sys/sys: mount.h vnode.h vnode_impl.h src/sys/ufs/lfs: ulfs_vfsops.c src/sys/ufs/ufs: ufs_vfsops.c ufs_wapbl.c Log Message: Make mntvnode_lock per-mount, and address false sharing of struct mount. This change broke kUBSan syzbot. The sanitizer is now very noisy as struct mount requires 64 byte alignment. http://netbsd.org/~kamil/kubsan/mount-alignment.txt I had a look this weekend. This is down to KMEM_SIZE messing up the alignment, so is a DIAGNOSTIC thing. The align_offset parameter to pool_cache() would be a nice easy way to solve this but it seems someone killed that off, so I'll need to give this some more thought. Andrew kmem guarantees alignment on 8 bytes, but not more. Changing the backend allocators to enforce more alignment still may not result in aligned buffers, because kmem has the right to modify the buffers for debugging purposes, like with KMEM_SIZE. If you want a buffer aligned to a specific value, don't use kmem, rather a pool(_cache) with COHERENCY_UNIT in "align". If the goal is to have kmem really enforce COHERENCY_UNIT alignment, then this should be documented and the debugging features should be adapted to respect that constraint. "align_offset" got removed because it increased complexity in subr_pool for no reason (two users in all of the kernel, one was actually a bug).
Re: CVS commit: src/sys/arch/aarch64
Le 28/01/2020 à 19:39, Nick Hudson a écrit : > On 28/01/2020 17:47, Maxime Villard wrote: >> @@ -460,8 +460,7 @@ cpu_setup_id(struct cpu_info *ci) >> >> id->ac_aa64mmfr0 = reg_id_aa64mmfr0_el1_read(); >> id->ac_aa64mmfr1 = reg_id_aa64mmfr1_el1_read(); >> - /* Only in ARMv8.2. */ >> - id->ac_aa64mmfr2 = 0 /* reg_id_aa64mmfr2_el1_read() */; >> + id->ac_aa64mmfr2 = reg_id_aa64mmfr2_el1_read(); >> >> id->ac_mvfr0 = reg_mvfr0_el1_read(); >> id->ac_mvfr1 = reg_mvfr1_el1_read(); > > I didn't ok this bit... Verily you did; I sent you this patch as-is two weeks ago. > This needs to be conditional on the CPU we're running on. ID_AA64MMFR2_EL1 is res0 on < ARMv8.2, reading it is therefore not a problem.
Re: [x86 pmap changes] CVS commit: src/sys/arch
Le 08/01/2020 à 22:50, Andrew Doran a écrit : On Tue, Jan 07, 2020 at 09:39:22AM +0100, Maxime Villard wrote: Module Name:src Committed By: ad Date: Sat Jan 4 22:49:20 UTC 2020 Modified Files: src/sys/arch/x86/include: pmap.h pmap_pv.h src/sys/arch/x86/x86: pmap.c src/sys/arch/xen/x86: xen_pmap.c Log Message: x86 pmap improvements, reducing system time during a build by about 15% on my test machine: This breaks nvmm-intel. I have only given a quick glance, but this change already is wrong: - old_pp->pp_attrs |= pmap_ept_to_pp_attrs(opte); + old_pp->pp_attrs |= pmap_pte_to_pp_attrs(opte); This is an EPT function handling EPT PTEs, so "ept" was correct. Fixing this bug is not sufficient, so it seems that there are more bugs. Reverting the whole change puts nvmm-intel back in a functional state. You can test with this on an Intel CPU: # modload nvmm # /usr/tests/lib/libnvmm/./h_mem_assist This currently gives random crashes. With a couple of typos fixed (PTE -> EPT, now checked in) I see the same FPU DNA exception that Chavdar reports on current-users (in his case with a kernel which doesn't have these pmap changes). It's coming from: vmx_vcpu_guest_fpu_leave() -> fpu_area_save() -> fxsave() What I can tell you is that the fxsave area is definitely writable and correctly aligned but beyond that I have no idea what's causing it. Any suggestions? Cheers, Andrew This FPU issue should be fixed in the latest nvmm_x86_vmx.c, we still have STTS/CLTS (not needed but for debugging) as part of context switches, and when overhauling the FPU code I overlooked that VMX needs special CR0_TS care that SVM doesn't need. Note that dropping STTS/CLTS would probably increase cswitch performance, because updating cr0 is costly. Having said that, I am still hitting a KASSERT related to pmap: kernel diagnostic assertion "ptp->wire_count == 1" failed file ".../x86/x86/pmap.c", line 1969 pmap_freepages pmap_ept_free_ptp pmap_ept_remove pmap_remove uvm_unmap_remove uvmspace_free nvmm_ioctl sys_ioctl Maxime
Re: [x86 pmap changes] CVS commit: src/sys/arch
Le 07/01/2020 à 09:39, Maxime Villard a écrit : Module Name: src Committed By: ad Date: Sat Jan 4 22:49:20 UTC 2020 Modified Files: src/sys/arch/x86/include: pmap.h pmap_pv.h src/sys/arch/x86/x86: pmap.c src/sys/arch/xen/x86: xen_pmap.c Log Message: x86 pmap improvements, reducing system time during a build by about 15% on my test machine: This breaks nvmm-intel. I have only given a quick glance, but this change already is wrong: - old_pp->pp_attrs |= pmap_ept_to_pp_attrs(opte); + old_pp->pp_attrs |= pmap_pte_to_pp_attrs(opte); This is an EPT function handling EPT PTEs, so "ept" was correct. Fixing this bug is not sufficient, so it seems that there are more bugs. Reverting the whole change puts nvmm-intel back in a functional state. You can test with this on an Intel CPU: # modload nvmm # /usr/tests/lib/libnvmm/./h_mem_assist This currently gives random crashes. Maxime Also unrelated remark: + kmutex_t pm_lock/* locks for pm_objs */ + __aligned(64); /* give lock own cache line */ x86 CPUs will soon have 128-byte cache lines, and to ease the upgrade, it is better to use COHERENCY_UNIT rather than hardcoded values.
[x86 pmap changes] CVS commit: src/sys/arch
Module Name:src Committed By: ad Date: Sat Jan 4 22:49:20 UTC 2020 Modified Files: src/sys/arch/x86/include: pmap.h pmap_pv.h src/sys/arch/x86/x86: pmap.c src/sys/arch/xen/x86: xen_pmap.c Log Message: x86 pmap improvements, reducing system time during a build by about 15% on my test machine: This breaks nvmm-intel. I have only given a quick glance, but this change already is wrong: - old_pp->pp_attrs |= pmap_ept_to_pp_attrs(opte); + old_pp->pp_attrs |= pmap_pte_to_pp_attrs(opte); This is an EPT function handling EPT PTEs, so "ept" was correct. Fixing this bug is not sufficient, so it seems that there are more bugs. Reverting the whole change puts nvmm-intel back in a functional state. You can test with this on an Intel CPU: # modload nvmm # /usr/tests/lib/libnvmm/./h_mem_assist This currently gives random crashes. Maxime
Re: CVS commit: src/sys/arch/amd64
Le 05/01/2020 à 13:56, Maxime Villard a écrit : > Le 05/01/2020 à 02:03, Emmanuel Dreyfus a écrit : >> On Sat, Jan 04, 2020 at 08:43:16AM +0100, Maxime Villard wrote: >>> +.section multiboot,"",@note >>> Why @note? It will be in the .text anyway. Also why no dot in the section >>> name? That's supposed to be the naming convention. >> >> The idea is that one day if ld gets more reasonable, it could go in >> non-loading note ection at the beginning of the binary, but if you >> prefer .text, let us go with that. > > I think .text.multiboot is fine, and @note should be dropped > >> Attached is my latest change set, including the locore cleanup you asked >> for. > > Notice how, after cleanup, the big copy crap comes down to literally just > two instructions. Unfortunately that's not exactly it: > > - As I said more than three weeks ago [1], I think it's the whole >MULTIBOOT block that should be moved in a separate file, not just the >32bit copy function. Only the '.Lbegin' label (to be renamed) needs to >be in locore.S, the rest can (and should) be outside. > - multiboot2_pre_reloc_would_be_built_as_ia32 should be removed. > - The code is still not entirely KNF, search for "\t\n". > - Local labels should begin with ".L". > - Now I'm wondering why KEEP() in the ldscript? Why doesn't >"*(.text.multiboot)" suffice? > > And also... Recovering from the heart attack I got after looking at > multiboot2_copy_syms32, I'm a bit confused; did you just objdump the > function and copy-paste it in the kernel? How did you obtain this > code? [Is it normal that I am already worried about your next answer?] > > Overall, I'm irritated, yes, because instead of reverting the change and > taking just one peaceful hour to fix things correctly, you have decided to > waste everybody's time with the breakage and absurd patch-work. I find > myself having to _insist_ for you to clean up the junk, and now I'm even > quoting emails from one month ago. > > Maxime > > [1] https://mail-index.netbsd.org/source-changes-d/2019/12/12/msg011882.html Sorry, but not gonna waste more of my time. I have now requested to core@ that multiboot in amd64 be reverted entirely. The correct way to add multiboot can be discussed afterwards, when the patch will have been shared and will have been agreed upon beforehand, in a way that it at least isn't total junk and doesn't break other things. Maxime
Re: CVS commit: src/sys/arch/amd64
Le 05/01/2020 à 02:03, Emmanuel Dreyfus a écrit : > On Sat, Jan 04, 2020 at 08:43:16AM +0100, Maxime Villard wrote: >> +.section multiboot,"",@note >> Why @note? It will be in the .text anyway. Also why no dot in the section >> name? That's supposed to be the naming convention. > > The idea is that one day if ld gets more reasonable, it could go in > non-loading note ection at the beginning of the binary, but if you > prefer .text, let us go with that. I think .text.multiboot is fine, and @note should be dropped > Attached is my latest change set, including the locore cleanup you asked > for. Notice how, after cleanup, the big copy crap comes down to literally just two instructions. Unfortunately that's not exactly it: - As I said more than three weeks ago [1], I think it's the whole MULTIBOOT block that should be moved in a separate file, not just the 32bit copy function. Only the '.Lbegin' label (to be renamed) needs to be in locore.S, the rest can (and should) be outside. - multiboot2_pre_reloc_would_be_built_as_ia32 should be removed. - The code is still not entirely KNF, search for "\t\n". - Local labels should begin with ".L". - Now I'm wondering why KEEP() in the ldscript? Why doesn't "*(.text.multiboot)" suffice? And also... Recovering from the heart attack I got after looking at multiboot2_copy_syms32, I'm a bit confused; did you just objdump the function and copy-paste it in the kernel? How did you obtain this code? [Is it normal that I am already worried about your next answer?] Overall, I'm irritated, yes, because instead of reverting the change and taking just one peaceful hour to fix things correctly, you have decided to waste everybody's time with the breakage and absurd patch-work. I find myself having to _insist_ for you to clean up the junk, and now I'm even quoting emails from one month ago. Maxime [1] https://mail-index.netbsd.org/source-changes-d/2019/12/12/msg011882.html
Re: CVS commit: src/sys/arch/amd64
Le 04/01/2020 à 03:33, Emmanuel Dreyfus a écrit : On Tue, Dec 31, 2019 at 09:32:05AM +0100, Maxime Villard wrote: I think max-page-size=0x1000 is the right thing to do, but someone needs to verify that the resulting binary is correct and that the resulting in-memory layout is correct too. Attached is an updated patch with this approach. I tested at mine and it seems fine. Come on... "I tested and it seems fine"... Whatever. I have now verified that the resulting binary layout is correct, that the in-memory layout is correct, and that libsa doesn't do crazy things with this binary. As far as my concerns were concerned, the max-page-size change is good to go. The rest is confused: +.section multiboot,"",@note Why @note? It will be in the .text anyway. Also why no dot in the section name? That's supposed to be the naming convention. +EXTRA_LINKFLAGS= --split-by-file=0x10 -z max-page-size=0x1000 -r -d KASLR kernels to not have a PHDR, so this is not relevant -- there was a reason this parameter wasn't getting passed in the first place. +optionsMULTIBOOT # Multiboot support (see multiboot(8)) As said repeatedly, the option should be enabled only _after_ the garbage has been cleaned up. In fact, why don't you revert your change, fix it correctly locally, and then re-submit it? I don't know if you realize, but you landed a huge pile of crap in the middle of the amd64 locore, not only does this crap not work but it also breaks EFI boot, and for two weeks you've been wondering what's wrong in it and you have consistently proposed absurd workarounds. Please revert your change entirely and put back the code in a clean and functional state. Thanks. Maxime
Re: CVS commit: src/sys/arch/amd64
Le 30/12/2019 à 16:15, Emmanuel Dreyfus a écrit : > On Sat, Dec 28, 2019 at 02:22:21AM +, Emmanuel Dreyfus wrote: >>> Regardless of whether it is needed in this specific case, cutting the 2MBs >>> of zero in the binary is wanted. Unfortunately last I looked at this (two >>> years ago) there were some non-obvious consequences, and it needs to be >>> carefully done. >> >> Any hints about the problems you encountered? Perhaps we can work it >> around with an . = ALIGN(__LARGEE_PAGE_SIZE); before including .text.user ? > > No anwser here? It is difficult to address an unknown problem... Sorry for the delay. I don't remember the specifics, but it had to do with misaligned sections in the end. There is also some pretty retarded code in loadfile_elf32.c that should like some investigation, at least for this kind of change. I think max-page-size=0x1000 is the right thing to do, but someone needs to verify that the resulting binary is correct and that the resulting in-memory layout is correct too.
Re: CVS commit: src/sys/arch/amd64
Le 27/12/2019 à 17:45, Emmanuel Dreyfus a écrit : > On Fri, Dec 27, 2019 at 09:02:17AM +0100, Maxime Villard wrote: >> Please stop with the nonsense... In this patch you are making the multiboot >> header executable, and putting it in a section shared with userland under >> SVS. Neither should be required; more than that, both are absolutely _not_ >> wanted. > > What are the actual drawbacks? You are moving the structure to an area where it does not belong _at all_. We don't want to map things in userland for no reason. Now that I'm looking at the thing closely, I see why we are forced to make it executable. So, that's fine for that part. > FWIW, this is in line with how it was done on i386: it is just stored > at the beginning of .text. Xen does the same. Of course it seems more > natural to store that in a note section this is not loaded, but after > experimenting a lot, I am not sure it can be done, since ld really > want to push notes at the end of the file. Now that I'm looking at i386 I see you've indeed made the same nonsensical changes there, with all the unnecessary garbage in the code. To me, you only need .section .multiboot,"ax",@progbits and .text : AT (ADDR(.text) & 0x0fff) { + *(.multiboot) + . = ALIGN(__PAGE_SIZE); __text_user_start = . ; ... This guarantees that the structure is at the beginning of text. Regardless of whether it is needed in this specific case, cutting the 2MBs of zero in the binary is wanted. Unfortunately last I looked at this (two years ago) there were some non-obvious consequences, and it needs to be carefully done. Also, my previous remarks haven't been addressed entirely, and still stand. Maxime
Re: CVS commit: src/sys/arch/amd64
Le 26/12/2019 à 17:55, Emmanuel Dreyfus a écrit : > On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote: After this change, amd64 kernel does not boot on my HP Spectre x360 13-inch ae019TU laptop with pure UEFI boot mode. >> I have a UEFI boot machine and it also doesn't boot well. > > Please try the attached patch. > > It adds the -n flag to ld, which disable auto-alignment of sections > in the file. I undestand alignement is highly desirable for userland > programs that may be mapped from file, but useless for the kernel, > which is just readen once by the bootloader. > > Without auto-alignement, the .text segment starts right after the > ELF headers. This means the multiboot header can go in .text and > stay below 32k (as required by the multiboot specification). There > is no need for a multiboot section for that, and therefore no > need to modify the linker script. > > A side effect is that the kernel file shrinks of 2 MB, because there > is not an alignement hole between ELF headers and the .text section > anymore. > > My patch also enable the MULTIBOOT option so that we can check > nothing gets broken with it. You can also try with the option > disabled, of course. Please stop with the nonsense... In this patch you are making the multiboot header executable, and putting it in a section shared with userland under SVS. Neither should be required; more than that, both are absolutely _not_ wanted. Instead of trying to patch-work the thing over and over, you should probably revert it all, take an hour to peacefully write it correctly, and then submit it again. Given the way multiboot was written so far I don't see how we can accept to enable it by default. Maxime
Re: CVS commit: src
Le 23/12/2019 à 04:18, Taylor R Campbell a écrit : Date: Sun, 22 Dec 2019 10:24:01 +0100 From: Maxime Villard You, Martin Christos and Taylor, are trying to change subject, find excuses, and are sending me irrelevant responses vaguely insinuating that I should revert my change only without addressing the additional concerns expressed repeatedly. I fail to see whether I should take these as official core answers. If they are, please clearly say so. Please revert your filemon deletion and leave the existing mitigation in place for the moment pending discussion. We can address the additional concerns afterward. Thanks, -Riastradh, on behalf of core You see, it wasn't really complicated. I have now reverted the removal. Just to sum up a few points: - I am severely accused of wrongdoing. Yet, under core's watch, secteam proceeded with a careless disabling of filemon with no public discussion whatsoever. For some reason, there suddenly is no wrongdoing here. - Actually the reason is apparently that security issues must be discussed in private. Yet, having been in secteam, I was systematically told to stop the discussions and make them public, which I always did. For some reason, there is no need to make these discussions public anymore. - But whatever, I really have to revert my removal _before_ any form of discussion is engaged on this topic. Yet, for some reason, maya's broken disabling must be left as-is and shouldn't be reverted _after_ discussion on the topic. - All in the name of consensus and community. And who cares if core and secteam openly demonstrate gross disregard for said consensus and community the way they have just now. I believe core has just made a great farce of itself with these double standards. For the record: Le 21/12/2019 à 23:48, Christos Zoulas a écrit : If you are unhappy with core@ talk to board@. I have now escalated the matter to board. Maxime
Re: CVS commit: src
Le 21/12/2019 à 23:48, Christos Zoulas a écrit : In article <15520611-7273-9567-33a4-ff2490b2e...@m00nbsd.net>, Maxime Villard wrote: Le 21/12/2019 à 00:05, Taylor R Campbell a écrit : Security-team is not perfect. We're happy to discuss a better way to disable filemon provisionally, and/or how to better address the existing users if we are to delete it -- after you do as core asked you to do to resolve the interim dispute by restoring the tree. This is a social process. We can work together to make it better for everyone, but you have to be willing to work with the community, including accepting rulings by core to resolve disputes. I'm afraid you, Taylor, don't have a monopoly on representing the community. He does represent the community since he represents core. If you are unhappy with core@ talk to board@. If you are unhappy with core@ and board@, get the community to vote for you in the next elections, or get signatures to impeach them. They are the elected leadership of the project. It appears you didn't read correctly the line of mine you just quoted. To resolve this dispute, I have proposed to revert both my removal, and secteam's broken disabling. This gives a clear basis to start a discussion on what to do with filemon exactly. Is core fine with that? Or are there double standards at play here? Core has been very clear. They've asked you to revert your commits, not other commits. If they had been unhappy with other commits they would have asked the committers of the other commits to revert them. "If". Except that I haven't received any formal email from core. Why can't core just send a simple official email to say whether yes, or no, they are fine with me reverting secteam's change, in addition to mine? I have accepted core's ruling, but have since pointed out additional serious deficiencies with how filemon was dealt with, hence the concern on how to proceed. Core, however, has been unwilling to provide an answer and to work with the community. It's been two days of this, and I fail to see where the difficulty is. You, Martin Christos and Taylor, are trying to change subject, find excuses, and are sending me irrelevant responses vaguely insinuating that I should revert my change only without addressing the additional concerns expressed repeatedly. I fail to see whether I should take these as official core answers. If they are, please clearly say so. Maxime
Re: CVS commit: src
Le 21/12/2019 à 00:05, Taylor R Campbell a écrit : Security-team is not perfect. We're happy to discuss a better way to disable filemon provisionally, and/or how to better address the existing users if we are to delete it -- after you do as core asked you to do to resolve the interim dispute by restoring the tree. This is a social process. We can work together to make it better for everyone, but you have to be willing to work with the community, including accepting rulings by core to resolve disputes. I'm afraid you, Taylor, don't have a monopoly on representing the community. It just so happens that I, too, as a regular member and also as a main kernel developer, represent the community; and I don't think the community is really happy with how secteam disabled filemon without discussion. The community is likely even less happy with how it was disabled, considering that a quick discussion has already highlighted two apparent better ways to disable it. It appears that core and secteam failed to work properly with the community. To resolve this dispute, I have proposed to revert both my removal, and secteam's broken disabling. This gives a clear basis to start a discussion on what to do with filemon exactly. Is core fine with that? Or are there double standards at play here? Maxime
Re: CVS commit: src
Le 21/12/2019 à 10:50, m...@netbsd.org a écrit : On Fri, Dec 20, 2019 at 06:34:31PM -0800, John Nemeth wrote: I don't wish to get embroiled in this debate (even if I did start it by requesting the reversion). I just want to point out that there is a relatively simple way disable the autoloading of a module. From module(9): The directory from which the module is loaded will be searched for a file with the same name as the module file, but with the suffix ``.plist''. If this file is found, the prop_dictionary it contains will be loaded and passed to the module's modcmd() routine. If this prop_dictionary contains a ``noautoload'' property which is set to ``true'' then the system will refuse to load the module. The simplest way to do the above is: modload -p -b noautoload=true > .plist }-- End of excerpt from Maxime Villard The first proposed change was to create a .plist file. I liked it because it didn't require running postinstall to get the fix. It provoked a bit of disgruntled voices, a continuation tech-kern bikeshed about the correct way to disable module auto-loading in a per-module basis. Not installing the module allowed to avoid having to commit changes in a debated topic (even though I was just using the existing method). I was feeling rushed that we didn't commit something because of the excess discussion. I see. So you unilaterally pushed a controversial change in an expeditious move, without any form of actual public discussion and approval. Maxime
Re: CVS commit: src
Le 20/12/2019 à 20:52, Martin Husemann a écrit : On Fri, Dec 20, 2019 at 07:54:36PM +0100, Maxime Villard wrote: Alright, fair enough. I will revert my removal over the week-end, because it hasn't received sufficient public discussion. Thank you! As well, I will revert secteam's killing of the feature, because there has been no public discussion on that at all. Please do not. You *do* have a point here, but: 1) public discussion upfront for a security issue is not always possible, as you are well aware I'm afraid that's no excuse, in that several of the security issues in the past have had to be discussed publicly. (On your own personal insistence, by the way, and I see no reason why the policy would change all of a sudden just because you personally decided otherwise.) 2) there has been a public security advisory which assumes this change and would need to be revised in case of reversal This only means secteam doubled down in being wrong. Specifically, it seems to me that removing /dev/filemon would have been sufficient, instead of removing the kmod. People could re-create /dev/filemon with minimal effort, should they be interested in the feature. As opposed to that, rebuilding a kmod is a much bigger effort. So, in addition to the fact that the disabling of filemon hasn't been discussed at all, it seems to me it was disabled the wrong way. If you're serious one minute about doing things correctly with consensus, secteam's commit should be reverted immediately, especially considering that secteam's solution seems to be technically the least preferable, since we apparently care about the users of filemon. Obviously, if we were even more serious here, we would all understand that in the end filemon should be removed completely and that secteam's only mistake was to do only half the job. But you prefer to go the procedural bullshit way, so let's go down that path together. Regarding the SA, local DoSes are not considered security issues, yet for some reason the advisory mentions that as a security problem; the real problem with the races here is undefined behavior, which can separately lead to memory disclosures and privilege escalations, both being real security issues. Notice also the gross typo in the title; it's "escalation", people, not "escallation". Maxime
Re: CVS commit: src
Le 19/12/2019 à 17:57, Taylor R Campbell a écrit : Date: Thu, 19 Dec 2019 08:19:07 +0100 From: Maxime Villard I think you meant to say "REMOVING things you don't like". Correct, I made an editing error. Sorry for the confusion. In the meantime, I have absolutely no intent to reinstate filemon. You can reinstate it if you want, but it should come as no surprise to you in the near future that filemon, after whatever "necessary" discussion or different forms of bureaucratic idiocy you want to throw at it, will be deleted almost as fast as it came back from the attic. Especially considering the emails sent from the other people since I proceeded. This is not negotiable. It is core's position that the filemon removal should be undone for the time being, so please go ahead and undo it. Thanks, -Riastradh, on behalf of core Alright, fair enough. I will revert my removal over the week-end, because it hasn't received sufficient public discussion. As well, I will revert secteam's killing of the feature, because there has been no public discussion on that at all. Maxime
Re: CVS commit: src
Answering quickly: Le 18/12/2019 à 21:24, Taylor R Campbell a écrit : Module Name:src Committed By: maxv Date: Wed Dec 18 07:37:19 UTC 2019 Log Message: Retire filemon, discussed on tech-kern@. Please refrain from taking unilateral actions such as things you don't like without reasonable discussion. I think you meant to say "REMOVING things you don't like". I removed it after quick discussion, and taking into account the fact that filemon was completely killed by secteam with no discussion either. - Nobody skimming tech-kern by subject line would reasonably expect that a thread titled `[filemon] CVS commit: htdocs/support/security' is the place to find a proposal of deletion on 18h notice. See below - Nothing in the thread was actually a proposal for discussion -- just an announcement that you have taken a unilateral decision to remove it. The first paragraph of the first email of this thread is literally a question I'm asking to other people. "announcement of unilateral decision" is about as dishonest and mischaracterizing as it can get. Disappointing, but not unexpected. - And since the module is gone and the pseudo-device is disabled by default, no unilateral emergency action is warranted, even it turns out under discussion that there is community consensus that filemon should be removed from the tree. Maybe it could have occurred to you that it is precisely because this feature has been completely killed already that I moved forward and deleted it, after very quick discussion indeed? But see below Please revert the filemon deletion and ensure the tree builds within the next 24h. I have reverted the bmake change, because Maya's concern (about diff against FreeBSD) is legitimate, at least for the time being. If you want to see filemon deleted, you can raise the subject _after_ you have reverted the commits. You mean like, following the removal procedure core has already failed to abide by in the past? In fact, maybe we should agree once and for all on the actual procedure, and ALL accept to abide by it, INCLUDING core. What do you think? In the meantime, I have absolutely no intent to reinstate filemon. You can reinstate it if you want, but it should come as no surprise to you in the near future that filemon, after whatever "necessary" discussion or different forms of bureaucratic idiocy you want to throw at it, will be deleted almost as fast as it came back from the attic. Especially considering the emails sent from the other people since I proceeded. Maxime
Re: CVS commit: src/sys/arch
Le 12/12/2019 à 10:20, Maxime Villard a écrit : Le 10/12/2019 à 03:06, Emmanuel Dreyfus a écrit : Module Name: src Committed By: manu Date: Tue Dec 10 02:06:07 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: locore.S machdep.c src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript src/sys/arch/x86/x86: efi.c multiboot2.c Log Message: Add multiboot 2 support to amd64 kernel I don't see how this can work mov $(__kernel_end - kernel_text), %rcx /* size *. Malformed comment means the next instructions are commented, too. In addition, it seems that you inlined the whole bcopy.S file into locore.S, with all the unnecessary defines, duplicates and irrelevant comments. Why? It seems you could have just used a byte-by-byte copy, which would have avoided all the unnecessary garbage. Also, I would have put the whole #ifdefined(MULTIBOOT) block into a separate function, instead of inlining everything in start(), which makes the code hard to read. Or even better: in a separate file. IMO this commit needs to be revisited (and I see that the kern.ldscript change got reverted already). Maxime In addition: 745 movl$1,%eax 746 movl%eax,RELOC(multiboot2_enabled) multiboot2_enabled is a boolean, so it should be movb. Here we are overwriting the other booleans in memory. Then, later: 1634call_C_LABEL(multiboot2_post_reloc) Here, there should be a check on multiboot2_enabled before calling the function, not inside of it. This breaks shadow-based sanitizers, like kASan. I will disable this code for now.
Re: CVS commit: src/sys/arch
Le 10/12/2019 à 03:06, Emmanuel Dreyfus a écrit : Module Name:src Committed By: manu Date: Tue Dec 10 02:06:07 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: locore.S machdep.c src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript src/sys/arch/x86/x86: efi.c multiboot2.c Log Message: Add multiboot 2 support to amd64 kernel I don't see how this can work mov $(__kernel_end - kernel_text), %rcx /* size *. Malformed comment means the next instructions are commented, too. In addition, it seems that you inlined the whole bcopy.S file into locore.S, with all the unnecessary defines, duplicates and irrelevant comments. Why? It seems you could have just used a byte-by-byte copy, which would have avoided all the unnecessary garbage. Also, I would have put the whole #ifdefined(MULTIBOOT) block into a separate function, instead of inlining everything in start(), which makes the code hard to read. Or even better: in a separate file. IMO this commit needs to be revisited (and I see that the kern.ldscript change got reverted already). Maxime
Re: CVS commit: src/sys/kern
Le 08/12/2019 à 14:22, Martin Husemann a écrit : On Sun, Dec 08, 2019 at 12:58:20PM +0100, Maxime Villard wrote: kMSan has special constraints which, in this specific case, come down to: each function called from a KCOV instrumentation callback must be a static inline tagged with __nomsan. This was not the case with the updated in_interrupt(), but also still isn't the case with the lwp_getspecific() call, which will have to be dropped. This does not sound like a good reason to introduce MD code in sys/kern to me. Could should not be made worse to deal with sanitizer restrictions. Are there any alternatives? The clean way of doing this is having an MD kcov.h included from subr_kcov.c, like kASan. However, I expect that we will want a per-lwp kcov flag to indicate the current context (in exception, in interrupt, in nmi, etc), and when that happens, I don't expect that we will need in_interrupt anymore. Therefore, I wouldn't bother adding kcov.h headers, and rolling back to the previous version of in_interrupt for now is fine, considering that kcov currently has no use outside of amd64.
Re: CVS commit: src/sys/kern
Le 08/12/2019 à 00:51, Kamil Rytarowski a écrit : On 08.12.2019 00:35, matthew green wrote: Module Name:src Committed By: kamil Date: Sat Dec 7 19:50:34 UTC 2019 Modified Files: src/sys/kern: subr_kcov.c Log Message: Revert the in_interrupt() change to use again the x86 specific code This is prerequisite for kMSan and upcoming kernel changes. Discussed with why is this? what is the problem? kMSan has special constraints which, in this specific case, come down to: each function called from a KCOV instrumentation callback must be a static inline tagged with __nomsan. This was not the case with the updated in_interrupt(), but also still isn't the case with the lwp_getspecific() call, which will have to be dropped.
Re: CVS commit: src/sys
Le 06/12/2019 à 08:49, m...@netbsd.org a écrit : > On Fri, Dec 06, 2019 at 07:27:07AM +0000, Maxime Villard wrote: >> Log Message: >> Minor changes, reported by the LGTM bot. > > Would be nice if the commit message was "address some integer overflows" > or something. Except that it does not address integer overflows? Rather an undefined behavior if the pointer overflows; this would have probably caused an ugly crash instead of a clean panic. >> @@ -2205,7 +2205,7 @@ m_verify_packet(struct mbuf *m) >> >> dat = n->m_data; >> len = n->m_len; >> -if (__predict_false(dat + len < dat)) { >> +if (__predict_false(len < 0)) { >> panic("%s: incorrect length (len = %d)", __func__, len); >> } >> >> > > Hmm, was it trying to check that adding the two numbers together didn't > produce an overflow? (Not valid, but has a different meaning) It meant to test both whether len was negative or too big; now it just tests negative. Too big is actually useless here.
Re: CVS commit: src/sys/kern [change in kern_lwp.c]
Le 01/12/2019 à 16:27, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Sun Dec 1 15:27:58 UTC 2019 Modified Files: src/sys/kern: kern_lwp.c Log Message: Fix a longstanding problem with LWP limits. When changing the user's LWP count, we must use the process credentials because that's what the accounting entity is tied to. Reported-by: syzbot+d193266676f635661...@syzkaller.appspotmail.com Is there a mistake in the Reported-by? It corresponds to a report that was closed as a malformed duplicate of this original report: https://syzkaller.appspot.com/bug?id=8878f056a0a9cf0cc405f1926a4f236fdc721642 Also, the bug is still there apparently, if you want to give a look...
CVS commit: src/sys/netatalk
Module Name:src Committed By: maxv Date: Fri Nov 29 17:40:16 UTC 2019 Modified Files: src/sys/netatalk: ddp_usrreq.c Log Message: Add sanity check, only sat_len bytes got copied in, the rest is uninitialized. Found by KMSAN. To generate a diff of this commit: cvs rdiff -u -r1.73 -r1.74 src/sys/netatalk/ddp_usrreq.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/netatalk/ddp_usrreq.c diff -u src/sys/netatalk/ddp_usrreq.c:1.73 src/sys/netatalk/ddp_usrreq.c:1.74 --- src/sys/netatalk/ddp_usrreq.c:1.73 Sun Feb 24 07:20:33 2019 +++ src/sys/netatalk/ddp_usrreq.c Fri Nov 29 17:40:16 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: ddp_usrreq.c,v 1.73 2019/02/24 07:20:33 maxv Exp $ */ +/* $NetBSD: ddp_usrreq.c,v 1.74 2019/11/29 17:40:16 maxv Exp $ */ /* * Copyright (c) 1990,1991 Regents of The University of Michigan. @@ -27,7 +27,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ddp_usrreq.c,v 1.73 2019/02/24 07:20:33 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ddp_usrreq.c,v 1.74 2019/11/29 17:40:16 maxv Exp $"); #include "opt_mbuftrace.h" #include "opt_atalk.h" @@ -97,6 +97,8 @@ at_pcbsetaddr(struct ddpcb *ddp, struct if (sat->sat_family != AF_APPLETALK) return (EAFNOSUPPORT); + if (sat->sat_len != sizeof(*sat)) + return EINVAL; if (sat->sat_addr.s_node != ATADDR_ANYNODE || sat->sat_addr.s_net != ATADDR_ANYNET) {
CVS commit: src/sys/netatalk
Module Name:src Committed By: maxv Date: Fri Nov 29 17:40:16 UTC 2019 Modified Files: src/sys/netatalk: ddp_usrreq.c Log Message: Add sanity check, only sat_len bytes got copied in, the rest is uninitialized. Found by KMSAN. To generate a diff of this commit: cvs rdiff -u -r1.73 -r1.74 src/sys/netatalk/ddp_usrreq.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/dev
Module Name:src Committed By: maxv Date: Thu Nov 28 17:09:10 UTC 2019 Modified Files: src/sys/dev/pci: if_et.c if_msk.c if_sk.c mpii.c src/sys/dev/pcmcia: if_xi.c src/sys/dev/usb: if_atu.c if_urtw.c if_zyd.c Log Message: localify To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/dev/pci/if_et.c cvs rdiff -u -r1.94 -r1.95 src/sys/dev/pci/if_msk.c cvs rdiff -u -r1.101 -r1.102 src/sys/dev/pci/if_sk.c cvs rdiff -u -r1.23 -r1.24 src/sys/dev/pci/mpii.c cvs rdiff -u -r1.91 -r1.92 src/sys/dev/pcmcia/if_xi.c cvs rdiff -u -r1.65 -r1.66 src/sys/dev/usb/if_atu.c cvs rdiff -u -r1.19 -r1.20 src/sys/dev/usb/if_urtw.c cvs rdiff -u -r1.52 -r1.53 src/sys/dev/usb/if_zyd.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/pci/if_et.c diff -u src/sys/dev/pci/if_et.c:1.27 src/sys/dev/pci/if_et.c:1.28 --- src/sys/dev/pci/if_et.c:1.27 Sat Oct 12 06:00:52 2019 +++ src/sys/dev/pci/if_et.c Thu Nov 28 17:09:10 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: if_et.c,v 1.27 2019/10/12 06:00:52 msaitoh Exp $ */ +/* $NetBSD: if_et.c,v 1.28 2019/11/28 17:09:10 maxv Exp $ */ /* $OpenBSD: if_et.c,v 1.12 2008/07/11 09:29:02 kevlo $ */ /* * Copyright (c) 2007 The DragonFly Project. All rights reserved. @@ -37,7 +37,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: if_et.c,v 1.27 2019/10/12 06:00:52 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_et.c,v 1.28 2019/11/28 17:09:10 maxv Exp $"); #include "opt_inet.h" #include "vlan.h" @@ -81,63 +81,62 @@ __KERNEL_RCSID(0, "$NetBSD: if_et.c,v 1. #include -int et_match(device_t, cfdata_t, void *); -void et_attach(device_t, device_t, void *); -int et_detach(device_t, int); -int et_shutdown(device_t); - -int et_miibus_readreg(device_t, int, int, uint16_t *); -int et_miibus_writereg(device_t, int, int, uint16_t); -void et_miibus_statchg(struct ifnet *); - -int et_init(struct ifnet *); -int et_ioctl(struct ifnet *, u_long, void *); -void et_start(struct ifnet *); -void et_watchdog(struct ifnet *); +static int et_match(device_t, cfdata_t, void *); +static void et_attach(device_t, device_t, void *); +static int et_detach(device_t, int); + +static int et_miibus_readreg(device_t, int, int, uint16_t *); +static int et_miibus_writereg(device_t, int, int, uint16_t); +static void et_miibus_statchg(struct ifnet *); + +static int et_init(struct ifnet *); +static int et_ioctl(struct ifnet *, u_long, void *); +static void et_start(struct ifnet *); +static void et_watchdog(struct ifnet *); static int et_ifmedia_upd(struct ifnet *); static void et_ifmedia_sts(struct ifnet *, struct ifmediareq *); -int et_intr(void *); -void et_enable_intrs(struct et_softc *, uint32_t); -void et_disable_intrs(struct et_softc *); -void et_rxeof(struct et_softc *); -void et_txeof(struct et_softc *); -void et_txtick(void *); - -int et_dma_alloc(struct et_softc *); -void et_dma_free(struct et_softc *); -int et_dma_mem_create(struct et_softc *, bus_size_t, +static int et_intr(void *); +static void et_enable_intrs(struct et_softc *, uint32_t); +static void et_disable_intrs(struct et_softc *); +static void et_rxeof(struct et_softc *); +static void et_txeof(struct et_softc *); +static void et_txtick(void *); + +static int et_dma_alloc(struct et_softc *); +static void et_dma_free(struct et_softc *); +static int et_dma_mem_create(struct et_softc *, bus_size_t, void **, bus_addr_t *, bus_dmamap_t *, bus_dma_segment_t *); -void et_dma_mem_destroy(struct et_softc *, void *, bus_dmamap_t); -int et_dma_mbuf_create(struct et_softc *); -void et_dma_mbuf_destroy(struct et_softc *, int, const int[]); - -int et_init_tx_ring(struct et_softc *); -int et_init_rx_ring(struct et_softc *); -void et_free_tx_ring(struct et_softc *); -void et_free_rx_ring(struct et_softc *); -int et_encap(struct et_softc *, struct mbuf **); -int et_newbuf(struct et_rxbuf_data *, int, int, int); -int et_newbuf_cluster(struct et_rxbuf_data *, int, int); -int et_newbuf_hdr(struct et_rxbuf_data *, int, int); - -void et_stop(struct et_softc *); -int et_chip_init(struct et_softc *); -void et_chip_attach(struct et_softc *); -void et_init_mac(struct et_softc *); -void et_init_rxmac(struct et_softc *); -void et_init_txmac(struct et_softc *); -int et_init_rxdma(struct et_softc *); -int et_init_txdma(struct et_softc *); -int et_start_rxdma(struct et_softc *); -int et_start_txdma(struct et_softc *); -int et_stop_rxdma(struct et_softc *); -int et_stop_txdma(struct et_softc *); -void et_reset(struct et_softc *); -int et_bus_config(struct et_softc *); -void et_get_eaddr(struct et_softc *, uint8_t[]); -void et_setmulti(struct et_softc *); -void et_tick(void *); +static void et_dma_mem_destroy(struct et_softc *, void *, bus_dmamap_t); +static int et_dma_mbuf_create(struct et_softc *); +static void et_dma_mbuf_destroy(struct et_softc *, int, const int[]); + +static int et_init_tx_ring(struct et_softc *); +static int
CVS commit: src/sys/dev
Module Name:src Committed By: maxv Date: Thu Nov 28 17:09:10 UTC 2019 Modified Files: src/sys/dev/pci: if_et.c if_msk.c if_sk.c mpii.c src/sys/dev/pcmcia: if_xi.c src/sys/dev/usb: if_atu.c if_urtw.c if_zyd.c Log Message: localify To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/dev/pci/if_et.c cvs rdiff -u -r1.94 -r1.95 src/sys/dev/pci/if_msk.c cvs rdiff -u -r1.101 -r1.102 src/sys/dev/pci/if_sk.c cvs rdiff -u -r1.23 -r1.24 src/sys/dev/pci/mpii.c cvs rdiff -u -r1.91 -r1.92 src/sys/dev/pcmcia/if_xi.c cvs rdiff -u -r1.65 -r1.66 src/sys/dev/usb/if_atu.c cvs rdiff -u -r1.19 -r1.20 src/sys/dev/usb/if_urtw.c cvs rdiff -u -r1.52 -r1.53 src/sys/dev/usb/if_zyd.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Wed Nov 27 19:21:37 UTC 2019 Modified Files: src/sys/arch/x86/pci: if_vmx.c src/sys/dev/pci: mfii.c Log Message: localify To generate a diff of this commit: cvs rdiff -u -r1.51 -r1.52 src/sys/arch/x86/pci/if_vmx.c cvs rdiff -u -r1.5 -r1.6 src/sys/dev/pci/mfii.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/x86/pci/if_vmx.c diff -u src/sys/arch/x86/pci/if_vmx.c:1.51 src/sys/arch/x86/pci/if_vmx.c:1.52 --- src/sys/arch/x86/pci/if_vmx.c:1.51 Thu Oct 10 08:55:08 2019 +++ src/sys/arch/x86/pci/if_vmx.c Wed Nov 27 19:21:36 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: if_vmx.c,v 1.51 2019/10/10 08:55:08 knakahara Exp $ */ +/* $NetBSD: if_vmx.c,v 1.52 2019/11/27 19:21:36 maxv Exp $ */ /* $OpenBSD: if_vmx.c,v 1.16 2014/01/22 06:04:17 brad Exp $ */ /* @@ -19,7 +19,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.51 2019/10/10 08:55:08 knakahara Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.52 2019/11/27 19:21:36 maxv Exp $"); #include #include @@ -340,118 +340,117 @@ typedef enum { #define vtophys(va) 0 /* XXX ok? */ -int vmxnet3_match(device_t, cfdata_t, void *); -void vmxnet3_attach(device_t, device_t, void *); -int vmxnet3_detach(device_t, int); - -int vmxnet3_alloc_pci_resources(struct vmxnet3_softc *); -void vmxnet3_free_pci_resources(struct vmxnet3_softc *); -int vmxnet3_check_version(struct vmxnet3_softc *); -void vmxnet3_check_multiqueue(struct vmxnet3_softc *); - -int vmxnet3_alloc_msix_interrupts(struct vmxnet3_softc *); -int vmxnet3_alloc_msi_interrupts(struct vmxnet3_softc *); -int vmxnet3_alloc_legacy_interrupts(struct vmxnet3_softc *); -int vmxnet3_alloc_interrupts(struct vmxnet3_softc *); -void vmxnet3_free_interrupts(struct vmxnet3_softc *); - -int vmxnet3_setup_msix_interrupts(struct vmxnet3_softc *); -int vmxnet3_setup_msi_interrupt(struct vmxnet3_softc *); -int vmxnet3_setup_legacy_interrupt(struct vmxnet3_softc *); -void vmxnet3_set_interrupt_idx(struct vmxnet3_softc *); -int vmxnet3_setup_interrupts(struct vmxnet3_softc *); -int vmxnet3_setup_sysctl(struct vmxnet3_softc *); - -int vmxnet3_setup_stats(struct vmxnet3_softc *); -void vmxnet3_teardown_stats(struct vmxnet3_softc *); - -int vmxnet3_init_rxq(struct vmxnet3_softc *, int); -int vmxnet3_init_txq(struct vmxnet3_softc *, int); -int vmxnet3_alloc_rxtx_queues(struct vmxnet3_softc *); -void vmxnet3_destroy_rxq(struct vmxnet3_rxqueue *); -void vmxnet3_destroy_txq(struct vmxnet3_txqueue *); -void vmxnet3_free_rxtx_queues(struct vmxnet3_softc *); - -int vmxnet3_alloc_shared_data(struct vmxnet3_softc *); -void vmxnet3_free_shared_data(struct vmxnet3_softc *); -int vmxnet3_alloc_txq_data(struct vmxnet3_softc *); -void vmxnet3_free_txq_data(struct vmxnet3_softc *); -int vmxnet3_alloc_rxq_data(struct vmxnet3_softc *); -void vmxnet3_free_rxq_data(struct vmxnet3_softc *); -int vmxnet3_alloc_queue_data(struct vmxnet3_softc *); -void vmxnet3_free_queue_data(struct vmxnet3_softc *); -int vmxnet3_alloc_mcast_table(struct vmxnet3_softc *); -void vmxnet3_free_mcast_table(struct vmxnet3_softc *); -void vmxnet3_init_shared_data(struct vmxnet3_softc *); -void vmxnet3_reinit_rss_shared_data(struct vmxnet3_softc *); -void vmxnet3_reinit_shared_data(struct vmxnet3_softc *); -int vmxnet3_alloc_data(struct vmxnet3_softc *); -void vmxnet3_free_data(struct vmxnet3_softc *); -int vmxnet3_setup_interface(struct vmxnet3_softc *); - -void vmxnet3_evintr(struct vmxnet3_softc *); -bool vmxnet3_txq_eof(struct vmxnet3_txqueue *, u_int); -int vmxnet3_newbuf(struct vmxnet3_softc *, struct vmxnet3_rxqueue *, +static int vmxnet3_match(device_t, cfdata_t, void *); +static void vmxnet3_attach(device_t, device_t, void *); +static int vmxnet3_detach(device_t, int); + +static int vmxnet3_alloc_pci_resources(struct vmxnet3_softc *); +static void vmxnet3_free_pci_resources(struct vmxnet3_softc *); +static int vmxnet3_check_version(struct vmxnet3_softc *); +static void vmxnet3_check_multiqueue(struct vmxnet3_softc *); + +static int vmxnet3_alloc_msix_interrupts(struct vmxnet3_softc *); +static int vmxnet3_alloc_msi_interrupts(struct vmxnet3_softc *); +static int vmxnet3_alloc_legacy_interrupts(struct vmxnet3_softc *); +static int vmxnet3_alloc_interrupts(struct vmxnet3_softc *); +static void vmxnet3_free_interrupts(struct vmxnet3_softc *); + +static int vmxnet3_setup_msix_interrupts(struct vmxnet3_softc *); +static int vmxnet3_setup_msi_interrupt(struct vmxnet3_softc *); +static int vmxnet3_setup_legacy_interrupt(struct vmxnet3_softc *); +static void vmxnet3_set_interrupt_idx(struct vmxnet3_softc *); +static int vmxnet3_setup_interrupts(struct vmxnet3_softc *); +static int vmxnet3_setup_sysctl(struct vmxnet3_softc *); + +static int vmxnet3_setup_stats(struct vmxnet3_softc *); +static void vmxnet3_teardown_stats(struct vmxnet3_softc *); + +static int vmxnet3_init_rxq(struct
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Wed Nov 27 19:21:37 UTC 2019 Modified Files: src/sys/arch/x86/pci: if_vmx.c src/sys/dev/pci: mfii.c Log Message: localify To generate a diff of this commit: cvs rdiff -u -r1.51 -r1.52 src/sys/arch/x86/pci/if_vmx.c cvs rdiff -u -r1.5 -r1.6 src/sys/dev/pci/mfii.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/arch/x86
Module Name:src Committed By: maxv Date: Wed Nov 27 06:24:33 UTC 2019 Modified Files: src/sys/arch/x86/include: cpu.h fpu.h src/sys/arch/x86/x86: cpu.c fpu.c Log Message: Add a small API for in-kernel FPU operations. fpu_kern_enter(); /* do FPU stuff */ fpu_kern_leave(); To generate a diff of this commit: cvs rdiff -u -r1.113 -r1.114 src/sys/arch/x86/include/cpu.h cvs rdiff -u -r1.19 -r1.20 src/sys/arch/x86/include/fpu.h cvs rdiff -u -r1.176 -r1.177 src/sys/arch/x86/x86/cpu.c cvs rdiff -u -r1.59 -r1.60 src/sys/arch/x86/x86/fpu.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/x86/include/cpu.h diff -u src/sys/arch/x86/include/cpu.h:1.113 src/sys/arch/x86/include/cpu.h:1.114 --- src/sys/arch/x86/include/cpu.h:1.113 Sat Nov 23 19:40:37 2019 +++ src/sys/arch/x86/include/cpu.h Wed Nov 27 06:24:33 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cpu.h,v 1.113 2019/11/23 19:40:37 ad Exp $ */ +/* $NetBSD: cpu.h,v 1.114 2019/11/27 06:24:33 maxv Exp $ */ /* * Copyright (c) 1990 The Regents of the University of California. @@ -139,6 +139,8 @@ struct cpu_info { uintptr_t ci_pmap_data[64 / sizeof(uintptr_t)]; struct kcpuset *ci_tlb_cpuset; + int ci_kfpu_spl; + #ifndef XENPV struct intrsource *ci_isources[MAX_INTR_SOURCES]; #endif Index: src/sys/arch/x86/include/fpu.h diff -u src/sys/arch/x86/include/fpu.h:1.19 src/sys/arch/x86/include/fpu.h:1.20 --- src/sys/arch/x86/include/fpu.h:1.19 Sat Oct 12 06:31:03 2019 +++ src/sys/arch/x86/include/fpu.h Wed Nov 27 06:24:33 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: fpu.h,v 1.19 2019/10/12 06:31:03 maxv Exp $ */ +/* $NetBSD: fpu.h,v 1.20 2019/11/27 06:24:33 maxv Exp $ */ #ifndef _X86_FPU_H_ #define _X86_FPU_H_ @@ -30,6 +30,9 @@ void fpu_sigreset(struct lwp *); void fpu_lwp_fork(struct lwp *, struct lwp *); void fpu_lwp_abandon(struct lwp *l); +void fpu_kern_enter(void); +void fpu_kern_leave(void); + void process_write_fpregs_xmm(struct lwp *, const struct fxsave *); void process_write_fpregs_s87(struct lwp *, const struct save87 *); Index: src/sys/arch/x86/x86/cpu.c diff -u src/sys/arch/x86/x86/cpu.c:1.176 src/sys/arch/x86/x86/cpu.c:1.177 --- src/sys/arch/x86/x86/cpu.c:1.176 Sat Nov 23 19:40:37 2019 +++ src/sys/arch/x86/x86/cpu.c Wed Nov 27 06:24:33 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cpu.c,v 1.176 2019/11/23 19:40:37 ad Exp $ */ +/* $NetBSD: cpu.c,v 1.177 2019/11/27 06:24:33 maxv Exp $ */ /* * Copyright (c) 2000-2012 NetBSD Foundation, Inc. @@ -62,7 +62,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.176 2019/11/23 19:40:37 ad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.177 2019/11/27 06:24:33 maxv Exp $"); #include "opt_ddb.h" #include "opt_mpbios.h" /* for MPDEBUG */ @@ -376,6 +376,7 @@ cpu_attach(device_t parent, device_t sel ci->ci_acpiid = caa->cpu_id; ci->ci_cpuid = caa->cpu_number; ci->ci_func = caa->cpu_func; + ci->ci_kfpu_spl = -1; aprint_normal("\n"); /* Must be before mi_cpu_attach(). */ Index: src/sys/arch/x86/x86/fpu.c diff -u src/sys/arch/x86/x86/fpu.c:1.59 src/sys/arch/x86/x86/fpu.c:1.60 --- src/sys/arch/x86/x86/fpu.c:1.59 Wed Oct 30 16:32:04 2019 +++ src/sys/arch/x86/x86/fpu.c Wed Nov 27 06:24:33 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: fpu.c,v 1.59 2019/10/30 16:32:04 maxv Exp $ */ +/* $NetBSD: fpu.c,v 1.60 2019/11/27 06:24:33 maxv Exp $ */ /* * Copyright (c) 2008, 2019 The NetBSD Foundation, Inc. All @@ -96,7 +96,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.59 2019/10/30 16:32:04 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.60 2019/11/27 06:24:33 maxv Exp $"); #include "opt_multiprocessor.h" @@ -146,14 +146,9 @@ fpu_lwp_area(struct lwp *l) return area; } -/* - * Bring curlwp's FPU state in memory. It will get installed back in the CPU - * when returning to userland. - */ -void -fpu_save(void) +static inline void +fpu_save_lwp(struct lwp *l) { - struct lwp *l = curlwp; struct pcb *pcb = lwp_getpcb(l); union savefpu *area = >pcb_savefpu; @@ -166,6 +161,16 @@ fpu_save(void) kpreempt_enable(); } +/* + * Bring curlwp's FPU state in memory. It will get installed back in the CPU + * when returning to userland. + */ +void +fpu_save(void) +{ + fpu_save_lwp(curlwp); +} + void fpuinit(struct cpu_info *ci) { @@ -338,6 +343,45 @@ fpu_lwp_abandon(struct lwp *l) /* -- */ +void +fpu_kern_enter(void) +{ + struct lwp *l = curlwp; + struct cpu_info *ci; + int s; + + s = splhigh(); + + ci = curcpu(); + KASSERT(ci->ci_kfpu_spl == -1); + ci->ci_kfpu_spl = s; + + /* + * If we are in a softint and have a pinned lwp, the fpu state is that + * of the pinned lwp, so save it there. + */ + if ((l->l_pflag & LP_INTR) && (l->l_switchto != NULL)) { + fpu_save_lwp(l->l_switchto); + } else { + fpu_save_lwp(l); + } +} + +void +fpu_kern_leave(void) +{ + struct
CVS commit: src/sys/arch/x86
Module Name:src Committed By: maxv Date: Wed Nov 27 06:24:33 UTC 2019 Modified Files: src/sys/arch/x86/include: cpu.h fpu.h src/sys/arch/x86/x86: cpu.c fpu.c Log Message: Add a small API for in-kernel FPU operations. fpu_kern_enter(); /* do FPU stuff */ fpu_kern_leave(); To generate a diff of this commit: cvs rdiff -u -r1.113 -r1.114 src/sys/arch/x86/include/cpu.h cvs rdiff -u -r1.19 -r1.20 src/sys/arch/x86/include/fpu.h cvs rdiff -u -r1.176 -r1.177 src/sys/arch/x86/x86/cpu.c cvs rdiff -u -r1.59 -r1.60 src/sys/arch/x86/x86/fpu.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 22 14:28:46 UTC 2019 Modified Files: src/sys/kern: subr_msan.c src/sys/lib/libkern: libkern.h Log Message: Ah, strcat/strchr/strrchr are ASM functions, so instrument them. To generate a diff of this commit: cvs rdiff -u -r1.2 -r1.3 src/sys/kern/subr_msan.c cvs rdiff -u -r1.134 -r1.135 src/sys/lib/libkern/libkern.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/subr_msan.c diff -u src/sys/kern/subr_msan.c:1.2 src/sys/kern/subr_msan.c:1.3 --- src/sys/kern/subr_msan.c:1.2 Fri Nov 15 12:18:46 2019 +++ src/sys/kern/subr_msan.c Fri Nov 22 14:28:46 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: subr_msan.c,v 1.2 2019/11/15 12:18:46 maxv Exp $ */ +/* $NetBSD: subr_msan.c,v 1.3 2019/11/22 14:28:46 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -32,7 +32,7 @@ #define KMSAN_NO_INST #include -__KERNEL_RCSID(0, "$NetBSD: subr_msan.c,v 1.2 2019/11/15 12:18:46 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: subr_msan.c,v 1.3 2019/11/22 14:28:46 maxv Exp $"); #include #include @@ -187,7 +187,7 @@ kmsan_report_hook(const void *addr, size var = (char *)ptr + 4; strlcpy(buf, var, sizeof(buf)); var = buf; - fn = strchr(buf, '@'); + fn = __builtin_strchr(buf, '@'); *fn++ = '\0'; REPORT("MSan: Uninitialized %s Memory In %s() At Offset " "%zu, Variable '%s' From %s()\n", typename, hook, off, @@ -238,7 +238,7 @@ kmsan_report_inline(msan_orig_t orig, un var = (char *)ptr + 4; strlcpy(buf, var, sizeof(buf)); var = buf; - fn = strchr(buf, '@'); + fn = __builtin_strchr(buf, '@'); *fn++ = '\0'; REPORT("MSan: Uninitialized Variable '%s' From %s()\n", var, fn); @@ -754,6 +754,51 @@ kmsan_strlen(const char *str) return (s - str); } +char * +kmsan_strcat(char *dst, const char *src) +{ + size_t ldst, lsrc; + char *ret; + + kmsan_check_arg(sizeof(dst) + sizeof(src), "strcat"); + + ldst = __builtin_strlen(dst); + lsrc = __builtin_strlen(src); + kmsan_shadow_check(dst, ldst + 1, "strcat"); + kmsan_shadow_check(src, lsrc + 1, "strcat"); + ret = __builtin_strcat(dst, src); + kmsan_shadow_fill(dst, KMSAN_STATE_INITED, ldst + lsrc + 1); + + kmsan_init_ret(sizeof(char *)); + return ret; +} + +char * +kmsan_strchr(const char *s, int c) +{ + char *ret; + + kmsan_check_arg(sizeof(s) + sizeof(c), "strchr"); + kmsan_shadow_check(s, __builtin_strlen(s), "strchr"); + ret = __builtin_strchr(s, c); + + kmsan_init_ret(sizeof(char *)); + return ret; +} + +char * +kmsan_strrchr(const char *s, int c) +{ + char *ret; + + kmsan_check_arg(sizeof(s) + sizeof(c), "strrchr"); + kmsan_shadow_check(s, __builtin_strlen(s), "strrchr"); + ret = __builtin_strrchr(s, c); + + kmsan_init_ret(sizeof(char *)); + return ret; +} + #undef kcopy #undef copystr #undef copyin Index: src/sys/lib/libkern/libkern.h diff -u src/sys/lib/libkern/libkern.h:1.134 src/sys/lib/libkern/libkern.h:1.135 --- src/sys/lib/libkern/libkern.h:1.134 Thu Nov 14 16:23:53 2019 +++ src/sys/lib/libkern/libkern.h Fri Nov 22 14:28:46 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: libkern.h,v 1.134 2019/11/14 16:23:53 maxv Exp $ */ +/* $NetBSD: libkern.h,v 1.135 2019/11/22 14:28:46 maxv Exp $ */ /*- * Copyright (c) 1992, 1993 @@ -438,13 +438,22 @@ size_t kmsan_strlen(const char *); #endif /* These exist in GCC 3.x, but we don't bother. */ +#if defined(_KERNEL) && defined(KMSAN) +char *kmsan_strcat(char *, const char *); +char *kmsan_strchr(const char *, int); +char *kmsan_strrchr(const char *, int); +#define strcat(d, s) kmsan_strcat(d, s) +#define strchr(s, c) kmsan_strchr(s, c) +#define strrchr(s, c) kmsan_strrchr(s, c) +#else char *strcat(char *, const char *); +char *strchr(const char *, int); +char *strrchr(const char *, int); +#endif size_t strcspn(const char *, const char *); char *strncpy(char *, const char *, size_t); char *strncat(char *, const char *, size_t); int strncmp(const char *, const char *, size_t); -char *strchr(const char *, int); -char *strrchr(const char *, int); char *strstr(const char *, const char *); char *strpbrk(const char *, const char *); size_t strspn(const char *, const char *);
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 22 14:28:46 UTC 2019 Modified Files: src/sys/kern: subr_msan.c src/sys/lib/libkern: libkern.h Log Message: Ah, strcat/strchr/strrchr are ASM functions, so instrument them. To generate a diff of this commit: cvs rdiff -u -r1.2 -r1.3 src/sys/kern/subr_msan.c cvs rdiff -u -r1.134 -r1.135 src/sys/lib/libkern/libkern.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/tests/lib/libnvmm
Module Name:src Committed By: maxv Date: Fri Nov 22 10:26:32 UTC 2019 Modified Files: src/tests/lib/libnvmm: h_mem_assist.c Log Message: Several improvements. In particular, reduce CS.limit, because Intel CPUs perform strict sanity checks, and the previous (too high) limit caused the VM entry to fail. To generate a diff of this commit: cvs rdiff -u -r1.17 -r1.18 src/tests/lib/libnvmm/h_mem_assist.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/tests/lib/libnvmm/h_mem_assist.c diff -u src/tests/lib/libnvmm/h_mem_assist.c:1.17 src/tests/lib/libnvmm/h_mem_assist.c:1.18 --- src/tests/lib/libnvmm/h_mem_assist.c:1.17 Sun Oct 27 07:08:15 2019 +++ src/tests/lib/libnvmm/h_mem_assist.c Fri Nov 22 10:26:32 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: h_mem_assist.c,v 1.17 2019/10/27 07:08:15 maxv Exp $ */ +/* $NetBSD: h_mem_assist.c,v 1.18 2019/11/22 10:26:32 maxv Exp $ */ /* * Copyright (c) 2018-2019 The NetBSD Foundation, Inc. @@ -113,7 +113,7 @@ run_machine(struct nvmm_machine *mach, s return; default: - printf("Invalid!\n"); + printf("Invalid VMEXIT: 0x%lx\n", exit->reason); return; } } @@ -177,22 +177,22 @@ extern uint8_t test_64bit_15_begin, test extern uint8_t test_64bit_16_begin, test_64bit_16_end; static const struct test tests64[] = { - { "test1 - MOV", _begin, _end, 0x3004, 0 }, - { "test2 - OR", _begin, _end, 0x16FF, 0 }, - { "test3 - AND", _begin, _end, 0x1FC0, 0 }, - { "test4 - XOR", _begin, _end, 0x10CF, 0 }, - { "test5 - Address Sizes", _begin, _end, 0x1F00, 0 }, - { "test6 - DMO", _begin, _end, 0xFFAB, 0 }, - { "test7 - STOS", _begin, _end, 0x00123456, 0 }, - { "test8 - LODS", _begin, _end, 0x12345678, 0 }, - { "test9 - MOVS", _begin, _end, 0x12345678, 0 }, - { "test10 - MOVZXB", _begin, _end, 0x0078, 0 }, - { "test11 - MOVZXW", _begin, _end, 0x5678, 0 }, - { "test12 - CMP", _begin, _end, 0x0001, 0 }, - { "test13 - SUB", _begin, _end, 0x000FA0FF, 0 }, - { "test14 - TEST", _begin, _end, 0x0001, 0 }, - { "test15 - XCHG", _64bit_15_begin, _64bit_15_end, 0x123456, 0 }, - { "test16 - XCHG", _64bit_16_begin, _64bit_16_end, + { "64bit test1 - MOV", _begin, _end, 0x3004, 0 }, + { "64bit test2 - OR", _begin, _end, 0x16FF, 0 }, + { "64bit test3 - AND", _begin, _end, 0x1FC0, 0 }, + { "64bit test4 - XOR", _begin, _end, 0x10CF, 0 }, + { "64bit test5 - Address Sizes", _begin, _end, 0x1F00, 0 }, + { "64bit test6 - DMO", _begin, _end, 0xFFAB, 0 }, + { "64bit test7 - STOS", _begin, _end, 0x00123456, 0 }, + { "64bit test8 - LODS", _begin, _end, 0x12345678, 0 }, + { "64bit test9 - MOVS", _begin, _end, 0x12345678, 0 }, + { "64bit test10 - MOVZXB", _begin, _end, 0x0078, 0 }, + { "64bit test11 - MOVZXW", _begin, _end, 0x5678, 0 }, + { "64bit test12 - CMP", _begin, _end, 0x0001, 0 }, + { "64bit test13 - SUB", _begin, _end, 0x000FA0FF, 0 }, + { "64bit test14 - TEST", _begin, _end, 0x0001, 0 }, + { "64bit test15 - XCHG", _64bit_15_begin, _64bit_15_end, 0x123456, 0 }, + { "64bit test16 - XCHG", _64bit_16_begin, _64bit_16_end, 0x123456, 0 }, { NULL, NULL, NULL, -1, 0 } }; @@ -218,6 +218,9 @@ reset_machine64(struct nvmm_machine *mac { struct nvmm_x64_state *state = vcpu->state; + if (nvmm_vcpu_getstate(mach, vcpu, NVMM_X64_STATE_ALL) == -1) + err(errno, "nvmm_vcpu_getstate"); + memset(state, 0, sizeof(*state)); /* Default. */ @@ -365,6 +368,8 @@ test_vm64(void) run_test(, , [i]); } + if (nvmm_vcpu_destroy(, ) == -1) + err(errno, "nvmm_vcpu_destroy"); if (nvmm_machine_destroy() == -1) err(errno, "nvmm_machine_destroy"); } @@ -400,10 +405,10 @@ reset_machine16(struct nvmm_machine *mac struct nvmm_x64_state *state = vcpu->state; if (nvmm_vcpu_getstate(mach, vcpu, NVMM_X64_STATE_ALL) == -1) - err(errno, "nvmm_vcpu_setstate"); + err(errno, "nvmm_vcpu_getstate"); state->segs[NVMM_X64_SEG_CS].base = 0; - state->segs[NVMM_X64_SEG_CS].limit = 0x; + state->segs[NVMM_X64_SEG_CS].limit = 0x2FFF; state->gprs[NVMM_X64_GPR_RIP] = 0x2000; if (nvmm_vcpu_setstate(mach, vcpu, NVMM_X64_STATE_ALL) == -1) @@ -451,6 +456,8 @@ test_vm16(void) run_test(, , [i]); } + if (nvmm_vcpu_destroy(, ) == -1) + err(errno, "nvmm_vcpu_destroy"); if (nvmm_machine_destroy() == -1) err(errno, "nvmm_machine_destroy"); }
CVS commit: src/tests/lib/libnvmm
Module Name:src Committed By: maxv Date: Fri Nov 22 10:26:32 UTC 2019 Modified Files: src/tests/lib/libnvmm: h_mem_assist.c Log Message: Several improvements. In particular, reduce CS.limit, because Intel CPUs perform strict sanity checks, and the previous (too high) limit caused the VM entry to fail. To generate a diff of this commit: cvs rdiff -u -r1.17 -r1.18 src/tests/lib/libnvmm/h_mem_assist.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/dev/nvmm/x86
Module Name:src Committed By: maxv Date: Wed Nov 20 10:26:56 UTC 2019 Modified Files: src/sys/dev/nvmm/x86: nvmm_x86_svm.c nvmm_x86_vmx.c Log Message: Hide XSAVES-specific stuff and the masked extended states. To generate a diff of this commit: cvs rdiff -u -r1.53 -r1.54 src/sys/dev/nvmm/x86/nvmm_x86_svm.c cvs rdiff -u -r1.44 -r1.45 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/nvmm/x86/nvmm_x86_svm.c diff -u src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.53 src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.54 --- src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.53 Mon Oct 28 08:30:49 2019 +++ src/sys/dev/nvmm/x86/nvmm_x86_svm.c Wed Nov 20 10:26:56 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: nvmm_x86_svm.c,v 1.53 2019/10/28 08:30:49 maxv Exp $ */ +/* $NetBSD: nvmm_x86_svm.c,v 1.54 2019/11/20 10:26:56 maxv Exp $ */ /* * Copyright (c) 2018-2019 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_svm.c,v 1.53 2019/10/28 08:30:49 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_svm.c,v 1.54 2019/11/20 10:26:56 maxv Exp $"); #include #include @@ -826,7 +826,18 @@ svm_inkernel_handle_cpuid(struct nvmm_cp cpudata->gprs[NVMM_X64_GPR_RDX] = svm_xcr0_mask >> 32; break; case 1: - cpudata->vmcb->state.rax &= ~CPUID_PES1_XSAVES; + cpudata->vmcb->state.rax &= + (CPUID_PES1_XSAVEOPT | CPUID_PES1_XSAVEC | + CPUID_PES1_XGETBV); + cpudata->gprs[NVMM_X64_GPR_RBX] = 0; + cpudata->gprs[NVMM_X64_GPR_RCX] = 0; + cpudata->gprs[NVMM_X64_GPR_RDX] = 0; + break; + default: + cpudata->vmcb->state.rax = 0; + cpudata->gprs[NVMM_X64_GPR_RBX] = 0; + cpudata->gprs[NVMM_X64_GPR_RCX] = 0; + cpudata->gprs[NVMM_X64_GPR_RDX] = 0; break; } break; Index: src/sys/dev/nvmm/x86/nvmm_x86_vmx.c diff -u src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.44 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.45 --- src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.44 Mon Oct 28 08:30:49 2019 +++ src/sys/dev/nvmm/x86/nvmm_x86_vmx.c Wed Nov 20 10:26:56 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: nvmm_x86_vmx.c,v 1.44 2019/10/28 08:30:49 maxv Exp $ */ +/* $NetBSD: nvmm_x86_vmx.c,v 1.45 2019/11/20 10:26:56 maxv Exp $ */ /* * Copyright (c) 2018-2019 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.44 2019/10/28 08:30:49 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.45 2019/11/20 10:26:56 maxv Exp $"); #include #include @@ -1201,7 +1201,18 @@ vmx_inkernel_handle_cpuid(struct nvmm_cp cpudata->gprs[NVMM_X64_GPR_RDX] = vmx_xcr0_mask >> 32; break; case 1: - cpudata->gprs[NVMM_X64_GPR_RAX] &= ~CPUID_PES1_XSAVES; + cpudata->gprs[NVMM_X64_GPR_RAX] &= + (CPUID_PES1_XSAVEOPT | CPUID_PES1_XSAVEC | + CPUID_PES1_XGETBV); + cpudata->gprs[NVMM_X64_GPR_RBX] = 0; + cpudata->gprs[NVMM_X64_GPR_RCX] = 0; + cpudata->gprs[NVMM_X64_GPR_RDX] = 0; + break; + default: + cpudata->gprs[NVMM_X64_GPR_RAX] = 0; + cpudata->gprs[NVMM_X64_GPR_RBX] = 0; + cpudata->gprs[NVMM_X64_GPR_RCX] = 0; + cpudata->gprs[NVMM_X64_GPR_RDX] = 0; break; } break;
CVS commit: src/sys/dev/nvmm/x86
Module Name:src Committed By: maxv Date: Wed Nov 20 10:26:56 UTC 2019 Modified Files: src/sys/dev/nvmm/x86: nvmm_x86_svm.c nvmm_x86_vmx.c Log Message: Hide XSAVES-specific stuff and the masked extended states. To generate a diff of this commit: cvs rdiff -u -r1.53 -r1.54 src/sys/dev/nvmm/x86/nvmm_x86_svm.c cvs rdiff -u -r1.44 -r1.45 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/arch/amd64
Module Name:src Committed By: maxv Date: Sun Nov 17 14:07:00 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: vector.S src/sys/arch/amd64/include: frameasm.h Log Message: Disable KCOV - by raising the interrupt level - in the TLB IPI handler, because this is only noise. To generate a diff of this commit: cvs rdiff -u -r1.70 -r1.71 src/sys/arch/amd64/amd64/vector.S cvs rdiff -u -r1.46 -r1.47 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/amd64/amd64/vector.S diff -u src/sys/arch/amd64/amd64/vector.S:1.70 src/sys/arch/amd64/amd64/vector.S:1.71 --- src/sys/arch/amd64/amd64/vector.S:1.70 Thu Mar 7 10:16:07 2019 +++ src/sys/arch/amd64/amd64/vector.S Sun Nov 17 14:07:00 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: vector.S,v 1.70 2019/03/07 10:16:07 nonaka Exp $ */ +/* $NetBSD: vector.S,v 1.71 2019/11/17 14:07:00 maxv Exp $ */ /* * Copyright (c) 1998, 2007, 2008 The NetBSD Foundation, Inc. @@ -316,7 +316,9 @@ IDTVEC_END(intr_hyperv_hypercall) IDTVEC(handle_lapic_tlb) movq _C_LABEL(local_apic_va),%rax movl $0,LAPIC_EOI(%rax) + KCOV_DISABLE callq _C_LABEL(pmap_tlb_intr) + KCOV_ENABLE INTRFASTEXIT IDTVEC_END(handle_lapic_tlb) IDTVEC(handle_x2apic_tlb) @@ -324,7 +326,9 @@ IDTVEC(handle_x2apic_tlb) xorl %eax,%eax xorl %edx,%edx wrmsr + KCOV_DISABLE callq _C_LABEL(pmap_tlb_intr) + KCOV_ENABLE INTRFASTEXIT IDTVEC_END(handle_x2apic_tlb) Index: src/sys/arch/amd64/include/frameasm.h diff -u src/sys/arch/amd64/include/frameasm.h:1.46 src/sys/arch/amd64/include/frameasm.h:1.47 --- src/sys/arch/amd64/include/frameasm.h:1.46 Thu Nov 14 16:23:52 2019 +++ src/sys/arch/amd64/include/frameasm.h Sun Nov 17 14:07:00 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: frameasm.h,v 1.46 2019/11/14 16:23:52 maxv Exp $ */ +/* $NetBSD: frameasm.h,v 1.47 2019/11/17 14:07:00 maxv Exp $ */ #ifndef _AMD64_MACHINE_FRAMEASM_H #define _AMD64_MACHINE_FRAMEASM_H @@ -6,6 +6,7 @@ #ifdef _KERNEL_OPT #include "opt_xen.h" #include "opt_svs.h" +#include "opt_kcov.h" #include "opt_kmsan.h" #endif @@ -267,6 +268,16 @@ #define KMSAN_INIT_RET(sz) /* nothing */ #endif +#ifdef KCOV +#define KCOV_DISABLE \ + incl CPUVAR(IDEPTH) +#define KCOV_ENABLE \ + decl CPUVAR(IDEPTH) +#else +#define KCOV_DISABLE /* nothing */ +#define KCOV_ENABLE /* nothing */ +#endif + #define INTRENTRY \ subq $TF_REGSIZE,%rsp ; \ INTR_SAVE_GPRS ; \
CVS commit: src/sys/arch/amd64
Module Name:src Committed By: maxv Date: Sun Nov 17 14:07:00 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: vector.S src/sys/arch/amd64/include: frameasm.h Log Message: Disable KCOV - by raising the interrupt level - in the TLB IPI handler, because this is only noise. To generate a diff of this commit: cvs rdiff -u -r1.70 -r1.71 src/sys/arch/amd64/amd64/vector.S cvs rdiff -u -r1.46 -r1.47 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/dev/usb
Module Name:src Committed By: maxv Date: Sun Nov 17 11:28:48 UTC 2019 Modified Files: src/sys/dev/usb: vhci.c Log Message: Not a bug strictly speaking, but compute the address only after the length checks, for clarity and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.3 -r1.4 src/sys/dev/usb/vhci.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/usb/vhci.c diff -u src/sys/dev/usb/vhci.c:1.3 src/sys/dev/usb/vhci.c:1.4 --- src/sys/dev/usb/vhci.c:1.3 Thu Oct 3 05:13:23 2019 +++ src/sys/dev/usb/vhci.c Sun Nov 17 11:28:48 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: vhci.c,v 1.3 2019/10/03 05:13:23 maxv Exp $ */ +/* $NetBSD: vhci.c,v 1.4 2019/11/17 11:28:48 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.3 2019/10/03 05:13:23 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vhci.c,v 1.4 2019/11/17 11:28:48 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -376,8 +376,6 @@ vhci_roothub_ctrl(struct usbd_bus *bus, value = UGETW(req->wValue); index = UGETW(req->wIndex); - port = >sc_port[VHCI_INDEX2PORT(index)]; - #define C(x,y) ((x) | ((y) << 8)) switch (C(req->bRequest, req->bmRequestType)) { case C(UR_GET_DESCRIPTOR, UT_READ_DEVICE): @@ -414,6 +412,7 @@ vhci_roothub_ctrl(struct usbd_bus *bus, if (index < 1 || index >= sc->sc_nports) { return -1; } + port = >sc_port[VHCI_INDEX2PORT(index)]; port->status |= UPS_C_PORT_RESET; break; case UHF_PORT_POWER: @@ -430,6 +429,7 @@ vhci_roothub_ctrl(struct usbd_bus *bus, if (index < 1 || index >= sc->sc_nports) { return -1; } + port = >sc_port[VHCI_INDEX2PORT(index)]; switch (value) { case UHF_PORT_ENABLE: port->status &= ~UPS_PORT_ENABLED; @@ -463,6 +463,7 @@ vhci_roothub_ctrl(struct usbd_bus *bus, if (index < 1 || index >= sc->sc_nports) { return -1; } + port = >sc_port[VHCI_INDEX2PORT(index)]; USETW(ps.wPortStatus, port->status); USETW(ps.wPortChange, port->change); totlen = uimin(len, sizeof(ps));
CVS commit: src/sys/dev/usb
Module Name:src Committed By: maxv Date: Sun Nov 17 11:28:48 UTC 2019 Modified Files: src/sys/dev/usb: vhci.c Log Message: Not a bug strictly speaking, but compute the address only after the length checks, for clarity and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.3 -r1.4 src/sys/dev/usb/vhci.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src [llvm]
Le 16/11/2019 à 23:01, Joerg Sonnenberger a écrit : On Tue, Nov 12, 2019 at 11:39:09AM +0100, Maxime Villard wrote: Le 11/11/2019 à 23:45, Joerg Sonnenberger a écrit : Module Name:src Committed By: joerg Date: Mon Nov 11 22:45:32 UTC 2019 [...] Log Message: Update LLVM to 10.0.0git (01f3a59fb3e2542fce74c768718f594d0debd0da) Since this change, I cannot cross-compile GENERIC amd64 from Linux: Problem for GCC 7 in NetBSD itself should be fixed. Please check if it works for your version too or otherwise report your GCC version. cross-compilation with GCC 9.2.1 on Linux still doesn't work
CVS commit: src/sys/dev/nvmm/x86
Module Name:src Committed By: maxv Date: Sat Nov 16 17:53:46 UTC 2019 Modified Files: src/sys/dev/nvmm/x86: nvmm_x86.c Log Message: Don't report MWAITX by default. To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/dev/nvmm/x86/nvmm_x86.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/nvmm/x86/nvmm_x86.c diff -u src/sys/dev/nvmm/x86/nvmm_x86.c:1.7 src/sys/dev/nvmm/x86/nvmm_x86.c:1.8 --- src/sys/dev/nvmm/x86/nvmm_x86.c:1.7 Wed May 15 04:39:52 2019 +++ src/sys/dev/nvmm/x86/nvmm_x86.c Sat Nov 16 17:53:46 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: nvmm_x86.c,v 1.7 2019/05/15 04:39:52 maxv Exp $ */ +/* $NetBSD: nvmm_x86.c,v 1.8 2019/11/16 17:53:46 maxv Exp $ */ /* * Copyright (c) 2018-2019 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: nvmm_x86.c,v 1.7 2019/05/15 04:39:52 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nvmm_x86.c,v 1.8 2019/11/16 17:53:46 maxv Exp $"); #include #include @@ -292,7 +292,7 @@ const struct nvmm_x86_cpuid_mask nvmm_cp .eax = ~0, .ebx = ~0, .ecx = - /* Excluded: SVM, EAPIC, OSVW. */ + /* Excluded: SVM, EAPIC, OSVW, MWAITX. */ CPUID_LAHF | CPUID_CMPLEGACY | CPUID_ALTMOVCR0 | CPUID_LZCNT | CPUID_SSE4A | CPUID_MISALIGNSSE | @@ -304,7 +304,7 @@ const struct nvmm_x86_cpuid_mask nvmm_cp CPUID_TOPOEXT | CPUID_PCEC | CPUID_PCENB | CPUID_SPM | CPUID_DBE | CPUID_PTSC | - CPUID_L2IPERFC | CPUID_MWAITX, + CPUID_L2IPERFC, .edx = /* Excluded: RDTSCP. */ CPUID_SYSCALL | CPUID_MPC |
CVS commit: src/sys/dev/nvmm/x86
Module Name:src Committed By: maxv Date: Sat Nov 16 17:53:46 UTC 2019 Modified Files: src/sys/dev/nvmm/x86: nvmm_x86.c Log Message: Don't report MWAITX by default. To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/dev/nvmm/x86/nvmm_x86.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/arch/x86/x86
Module Name:src Committed By: maxv Date: Sat Nov 16 10:19:29 UTC 2019 Modified Files: src/sys/arch/x86/x86: pmap.c Log Message: Add a NULL check on the structure pointer, not to retrieve its first field if it is NULL. The previous code was not buggy strictly speaking. This change probably doesn't change anything, except removing assumptions in the compiler optimization passes, which too probably doesn't change anything in this case. Reported-by: syzbot+110b29c1973f38a38...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.340 -r1.341 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/x86/x86/pmap.c diff -u src/sys/arch/x86/x86/pmap.c:1.340 src/sys/arch/x86/x86/pmap.c:1.341 --- src/sys/arch/x86/x86/pmap.c:1.340 Thu Nov 14 17:09:23 2019 +++ src/sys/arch/x86/x86/pmap.c Sat Nov 16 10:19:29 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: pmap.c,v 1.340 2019/11/14 17:09:23 maxv Exp $ */ +/* $NetBSD: pmap.c,v 1.341 2019/11/16 10:19:29 maxv Exp $ */ /* * Copyright (c) 2008, 2010, 2016, 2017 The NetBSD Foundation, Inc. @@ -130,7 +130,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.340 2019/11/14 17:09:23 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.341 2019/11/16 10:19:29 maxv Exp $"); #include "opt_user_ldt.h" #include "opt_lockdebug.h" @@ -541,6 +541,8 @@ static inline struct pv_pte * pve_to_pvpte(struct pv_entry *pve) { + if (pve == NULL) + return NULL; KASSERT((void *)>pve_pte == (void *)pve); return >pve_pte; }
CVS commit: src/sys/arch/x86/x86
Module Name:src Committed By: maxv Date: Sat Nov 16 10:19:29 UTC 2019 Modified Files: src/sys/arch/x86/x86: pmap.c Log Message: Add a NULL check on the structure pointer, not to retrieve its first field if it is NULL. The previous code was not buggy strictly speaking. This change probably doesn't change anything, except removing assumptions in the compiler optimization passes, which too probably doesn't change anything in this case. Reported-by: syzbot+110b29c1973f38a38...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.340 -r1.341 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/netinet
Module Name:src Committed By: maxv Date: Sat Nov 16 10:15:10 UTC 2019 Modified Files: src/sys/netinet: tcp_input.c Log Message: Call rtcache_unref() only when the checks succeed, instead of relying on another NULL check in rtcache_unref(). Because, in order to resolve the address of the second argument, we do a dereference on 'tp', which is theoretically allowed to be NULL. The five callers of nd6_hint() never pass a NULL argument however, so by luck the actual NULL deref never happens. Maybe the NULL check on 'tp' in should be replaced to a KASSERT ensuring it isn't NULL, for clarity. Reported by kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.416 -r1.417 src/sys/netinet/tcp_input.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/netinet
Module Name:src Committed By: maxv Date: Sat Nov 16 10:15:10 UTC 2019 Modified Files: src/sys/netinet: tcp_input.c Log Message: Call rtcache_unref() only when the checks succeed, instead of relying on another NULL check in rtcache_unref(). Because, in order to resolve the address of the second argument, we do a dereference on 'tp', which is theoretically allowed to be NULL. The five callers of nd6_hint() never pass a NULL argument however, so by luck the actual NULL deref never happens. Maybe the NULL check on 'tp' in should be replaced to a KASSERT ensuring it isn't NULL, for clarity. Reported by kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.416 -r1.417 src/sys/netinet/tcp_input.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/netinet/tcp_input.c diff -u src/sys/netinet/tcp_input.c:1.416 src/sys/netinet/tcp_input.c:1.417 --- src/sys/netinet/tcp_input.c:1.416 Wed Sep 25 19:06:30 2019 +++ src/sys/netinet/tcp_input.c Sat Nov 16 10:15:10 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: tcp_input.c,v 1.416 2019/09/25 19:06:30 jnemeth Exp $ */ +/* $NetBSD: tcp_input.c,v 1.417 2019/11/16 10:15:10 maxv Exp $ */ /* * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. @@ -148,7 +148,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tcp_input.c,v 1.416 2019/09/25 19:06:30 jnemeth Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tcp_input.c,v 1.417 2019/11/16 10:15:10 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_inet.h" @@ -260,9 +260,10 @@ nd6_hint(struct tcpcb *tp) struct rtentry *rt = NULL; if (tp != NULL && tp->t_in6pcb != NULL && tp->t_family == AF_INET6 && - (rt = rtcache_validate(>t_in6pcb->in6p_route)) != NULL) + (rt = rtcache_validate(>t_in6pcb->in6p_route)) != NULL) { nd6_nud_hint(rt); - rtcache_unref(rt, >t_in6pcb->in6p_route); + rtcache_unref(rt, >t_in6pcb->in6p_route); + } } #else static inline void
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Sat Nov 16 10:07:53 UTC 2019 Modified Files: src/sys/kern: vfs_mount.c Log Message: NULL-check the structure pointer, not the address of its first field. Also add KASSERT. For clarity, and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.71 -r1.72 src/sys/kern/vfs_mount.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Sat Nov 16 10:07:53 UTC 2019 Modified Files: src/sys/kern: vfs_mount.c Log Message: NULL-check the structure pointer, not the address of its first field. Also add KASSERT. For clarity, and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.71 -r1.72 src/sys/kern/vfs_mount.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/vfs_mount.c diff -u src/sys/kern/vfs_mount.c:1.71 src/sys/kern/vfs_mount.c:1.72 --- src/sys/kern/vfs_mount.c:1.71 Mon Aug 19 09:32:42 2019 +++ src/sys/kern/vfs_mount.c Sat Nov 16 10:07:53 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_mount.c,v 1.71 2019/08/19 09:32:42 christos Exp $ */ +/* $NetBSD: vfs_mount.c,v 1.72 2019/11/16 10:07:53 maxv Exp $ */ /*- * Copyright (c) 1997-2011 The NetBSD Foundation, Inc. @@ -67,7 +67,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: vfs_mount.c,v 1.71 2019/08/19 09:32:42 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_mount.c,v 1.72 2019/11/16 10:07:53 maxv Exp $"); #include #include @@ -420,11 +420,12 @@ vfs_vnode_iterator_next1(struct vnode_it TAILQ_REMOVE(>mnt_vnodelist, mvip, vi_mntvnodes); VIMPL_TO_VNODE(mvip)->v_usecount = 0; again: - vp = VIMPL_TO_VNODE(vip); - if (vp == NULL) { + if (vip == NULL) { mutex_exit(_lock); return NULL; } + vp = VIMPL_TO_VNODE(vip); + KASSERT(vp != NULL); mutex_enter(vp->v_interlock); if (vnis_marker(vp) || vdead_check(vp, (do_wait ? 0 : VDEAD_NOWAIT)) ||
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Sat Nov 16 10:05:44 UTC 2019 Modified Files: src/sys/kern: vfs_subr.c Log Message: Add a NULL check on the structure (same logic as my previous change in this file). For clarity, and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.473 -r1.474 src/sys/kern/vfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Sat Nov 16 10:05:44 UTC 2019 Modified Files: src/sys/kern: vfs_subr.c Log Message: Add a NULL check on the structure (same logic as my previous change in this file). For clarity, and to appease kUBSan. To generate a diff of this commit: cvs rdiff -u -r1.473 -r1.474 src/sys/kern/vfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/vfs_subr.c diff -u src/sys/kern/vfs_subr.c:1.473 src/sys/kern/vfs_subr.c:1.474 --- src/sys/kern/vfs_subr.c:1.473 Fri Nov 15 15:51:57 2019 +++ src/sys/kern/vfs_subr.c Sat Nov 16 10:05:44 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_subr.c,v 1.473 2019/11/15 15:51:57 maxv Exp $ */ +/* $NetBSD: vfs_subr.c,v 1.474 2019/11/16 10:05:44 maxv Exp $ */ /*- * Copyright (c) 1997, 1998, 2004, 2005, 2007, 2008 The NetBSD Foundation, Inc. @@ -68,7 +68,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.473 2019/11/15 15:51:57 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.474 2019/11/16 10:05:44 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_ddb.h" @@ -799,7 +799,8 @@ sched_sync(void *arg) * XXX The vnode may have been recycled, in which * case it may have a new identity. */ - if (VIMPL_TO_VNODE(TAILQ_FIRST(slp)) == vp) { + vi = TAILQ_FIRST(slp); + if (vi != NULL && VIMPL_TO_VNODE(vi) == vp) { /* * Put us back on the worklist. The worklist * routine will remove us from our current
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Fri Nov 15 15:51:57 UTC 2019 Modified Files: src/sys/kern: vfs_subr.c Log Message: NULL-check the structure pointer, not the address of its first field. This is clearer and also appeases syzbot. Reported-by: syzbot+d27bc1be926b3641c...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.472 -r1.473 src/sys/kern/vfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Fri Nov 15 15:51:57 UTC 2019 Modified Files: src/sys/kern: vfs_subr.c Log Message: NULL-check the structure pointer, not the address of its first field. This is clearer and also appeases syzbot. Reported-by: syzbot+d27bc1be926b3641c...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.472 -r1.473 src/sys/kern/vfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/vfs_subr.c diff -u src/sys/kern/vfs_subr.c:1.472 src/sys/kern/vfs_subr.c:1.473 --- src/sys/kern/vfs_subr.c:1.472 Sun Sep 22 22:59:39 2019 +++ src/sys/kern/vfs_subr.c Fri Nov 15 15:51:57 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_subr.c,v 1.472 2019/09/22 22:59:39 christos Exp $ */ +/* $NetBSD: vfs_subr.c,v 1.473 2019/11/15 15:51:57 maxv Exp $ */ /*- * Copyright (c) 1997, 1998, 2004, 2005, 2007, 2008 The NetBSD Foundation, Inc. @@ -68,7 +68,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.472 2019/09/22 22:59:39 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.473 2019/11/15 15:51:57 maxv Exp $"); #ifdef _KERNEL_OPT #include "opt_ddb.h" @@ -758,6 +758,7 @@ sched_sync(void *arg) { mount_iterator_t *iter; synclist_t *slp; + struct vnode_impl *vi; struct vnode *vp; struct mount *mp; time_t starttime; @@ -790,7 +791,8 @@ sched_sync(void *arg) if (syncer_delayno >= syncer_last) syncer_delayno = 0; - while ((vp = VIMPL_TO_VNODE(TAILQ_FIRST(slp))) != NULL) { + while ((vi = TAILQ_FIRST(slp)) != NULL) { + vp = VIMPL_TO_VNODE(vi); synced = lazy_sync_vnode(vp); /*
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 15 12:18:46 UTC 2019 Modified Files: src/sys/kern: subr_msan.c src/sys/sys: systm.h Log Message: Instrument ufetch/ustore in kMSan, these were the last remaining functions. To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/sys/kern/subr_msan.c cvs rdiff -u -r1.290 -r1.291 src/sys/sys/systm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/subr_msan.c diff -u src/sys/kern/subr_msan.c:1.1 src/sys/kern/subr_msan.c:1.2 --- src/sys/kern/subr_msan.c:1.1 Thu Nov 14 16:23:52 2019 +++ src/sys/kern/subr_msan.c Fri Nov 15 12:18:46 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: subr_msan.c,v 1.1 2019/11/14 16:23:52 maxv Exp $ */ +/* $NetBSD: subr_msan.c,v 1.2 2019/11/15 12:18:46 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -32,7 +32,7 @@ #define KMSAN_NO_INST #include -__KERNEL_RCSID(0, "$NetBSD: subr_msan.c,v 1.1 2019/11/14 16:23:52 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: subr_msan.c,v 1.2 2019/11/15 12:18:46 maxv Exp $"); #include #include @@ -870,6 +870,191 @@ kmsan_copyoutstr(const void *kaddr, void /* -- */ +#undef _ucas_32 +#undef _ucas_32_mp +#undef _ucas_64 +#undef _ucas_64_mp +#undef _ufetch_8 +#undef _ufetch_16 +#undef _ufetch_32 +#undef _ufetch_64 +#undef _ustore_8 +#undef _ustore_16 +#undef _ustore_32 +#undef _ustore_64 + +int _ucas_32(volatile uint32_t *, uint32_t, uint32_t, uint32_t *); +int kmsan__ucas_32(volatile uint32_t *, uint32_t, uint32_t, uint32_t *); +int +kmsan__ucas_32(volatile uint32_t *uaddr, uint32_t old, uint32_t new, +uint32_t *ret) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(old) + + sizeof(new) + sizeof(ret), "ucas_32"); + _ret = _ucas_32(uaddr, old, new, ret); + if (_ret == 0) + kmsan_shadow_fill(ret, KMSAN_STATE_INITED, sizeof(*ret)); + kmsan_init_ret(sizeof(int)); + return _ret; +} + +#ifdef __HAVE_UCAS_MP +int _ucas_32_mp(volatile uint32_t *, uint32_t, uint32_t, uint32_t *); +int kmsan__ucas_32_mp(volatile uint32_t *, uint32_t, uint32_t, uint32_t *); +int +kmsan__ucas_32_mp(volatile uint32_t *uaddr, uint32_t old, uint32_t new, +uint32_t *ret) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(old) + + sizeof(new) + sizeof(ret), "ucas_32_mp"); + _ret = _ucas_32_mp(uaddr, old, new, ret); + if (_ret == 0) + kmsan_shadow_fill(ret, KMSAN_STATE_INITED, sizeof(*ret)); + kmsan_init_ret(sizeof(int)); + return _ret; +} +#endif + +#ifdef _LP64 +int _ucas_64(volatile uint64_t *, uint64_t, uint64_t, uint64_t *); +int kmsan__ucas_64(volatile uint64_t *, uint64_t, uint64_t, uint64_t *); +int +kmsan__ucas_64(volatile uint64_t *uaddr, uint64_t old, uint64_t new, +uint64_t *ret) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(old) + + sizeof(new) + sizeof(ret), "ucas_64"); + _ret = _ucas_64(uaddr, old, new, ret); + if (_ret == 0) + kmsan_shadow_fill(ret, KMSAN_STATE_INITED, sizeof(*ret)); + kmsan_init_ret(sizeof(int)); + return _ret; +} + +#ifdef __HAVE_UCAS_MP +int _ucas_64_mp(volatile uint64_t *, uint64_t, uint64_t, uint64_t *); +int kmsan__ucas_64_mp(volatile uint64_t *, uint64_t, uint64_t, uint64_t *); +int +kmsan__ucas_64_mp(volatile uint64_t *uaddr, uint64_t old, uint64_t new, +uint64_t *ret) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(old) + + sizeof(new) + sizeof(ret), "ucas_64_mp"); + _ret = _ucas_64_mp(uaddr, old, new, ret); + if (_ret == 0) + kmsan_shadow_fill(ret, KMSAN_STATE_INITED, sizeof(*ret)); + kmsan_init_ret(sizeof(int)); + return _ret; +} +#endif +#endif + +int _ufetch_8(const uint8_t *, uint8_t *); +int kmsan__ufetch_8(const uint8_t *, uint8_t *); +int +kmsan__ufetch_8(const uint8_t *uaddr, uint8_t *valp) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(valp), "ufetch_8"); + _ret = _ufetch_8(uaddr, valp); + if (_ret == 0) + kmsan_shadow_fill(valp, KMSAN_STATE_INITED, sizeof(*valp)); + kmsan_init_ret(sizeof(int)); + return _ret; +} + +int _ufetch_16(const uint16_t *, uint16_t *); +int kmsan__ufetch_16(const uint16_t *, uint16_t *); +int +kmsan__ufetch_16(const uint16_t *uaddr, uint16_t *valp) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(valp), "ufetch_16"); + _ret = _ufetch_16(uaddr, valp); + if (_ret == 0) + kmsan_shadow_fill(valp, KMSAN_STATE_INITED, sizeof(*valp)); + kmsan_init_ret(sizeof(int)); + return _ret; +} + +int _ufetch_32(const uint32_t *, uint32_t *); +int kmsan__ufetch_32(const uint32_t *, uint32_t *); +int +kmsan__ufetch_32(const uint32_t *uaddr, uint32_t *valp) +{ + int _ret; + kmsan_check_arg(sizeof(uaddr) + sizeof(valp), "ufetch_32"); + _ret = _ufetch_32(uaddr, valp); + if (_ret == 0) + kmsan_shadow_fill(valp, KMSAN_STATE_INITED, sizeof(*valp)); + kmsan_init_ret(sizeof(int)); + return _ret; +} + +#ifdef _LP64 +int _ufetch_64(const uint64_t *, uint64_t *); +int
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 15 12:18:46 UTC 2019 Modified Files: src/sys/kern: subr_msan.c src/sys/sys: systm.h Log Message: Instrument ufetch/ustore in kMSan, these were the last remaining functions. To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/sys/kern/subr_msan.c cvs rdiff -u -r1.290 -r1.291 src/sys/sys/systm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/arch/amd64/amd64
Module Name:src Committed By: maxv Date: Fri Nov 15 09:50:01 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: cpu_in_cksum.S Log Message: Since cpu_in_cksum.S can be built outside of the kernel, add an ugly #ifdef _KERNEL for kMSan. To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/sys/arch/amd64/amd64/cpu_in_cksum.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/amd64/amd64/cpu_in_cksum.S diff -u src/sys/arch/amd64/amd64/cpu_in_cksum.S:1.4 src/sys/arch/amd64/amd64/cpu_in_cksum.S:1.5 --- src/sys/arch/amd64/amd64/cpu_in_cksum.S:1.4 Thu Nov 14 16:23:52 2019 +++ src/sys/arch/amd64/amd64/cpu_in_cksum.S Fri Nov 15 09:50:01 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cpu_in_cksum.S,v 1.4 2019/11/14 16:23:52 maxv Exp $ */ +/* $NetBSD: cpu_in_cksum.S,v 1.5 2019/11/15 09:50:01 maxv Exp $ */ /*- * Copyright (c) 2008 Joerg Sonnenberger . @@ -30,7 +30,11 @@ */ #include +#ifdef _KERNEL #include +#else +#define KMSAN_INIT_RET(sz) /* nothing */ +#endif #include "assym.h" ENTRY(cpu_in_cksum)
CVS commit: src/sys/arch/amd64/amd64
Module Name:src Committed By: maxv Date: Fri Nov 15 09:50:01 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: cpu_in_cksum.S Log Message: Since cpu_in_cksum.S can be built outside of the kernel, add an ugly #ifdef _KERNEL for kMSan. To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/sys/arch/amd64/amd64/cpu_in_cksum.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src
Module Name:src Committed By: maxv Date: Fri Nov 15 09:44:44 UTC 2019 Modified Files: src/share/mk: bsd.sys.mk src/sys/kern: subr_kcov.c Log Message: Make kMSan compatible with KCOV. With kMSan we are forced to stay with the fsanitize flag on subr_kcov.c, which means that kMSan will instrument KCOV. We add a bunch of __nomsan attributes to reduce this instrumentation, but it does not remove it completely. That's fine. To generate a diff of this commit: cvs rdiff -u -r1.295 -r1.296 src/share/mk/bsd.sys.mk cvs rdiff -u -r1.8 -r1.9 src/sys/kern/subr_kcov.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/share/mk/bsd.sys.mk diff -u src/share/mk/bsd.sys.mk:1.295 src/share/mk/bsd.sys.mk:1.296 --- src/share/mk/bsd.sys.mk:1.295 Tue Nov 5 20:19:17 2019 +++ src/share/mk/bsd.sys.mk Fri Nov 15 09:44:44 2019 @@ -1,4 +1,4 @@ -# $NetBSD: bsd.sys.mk,v 1.295 2019/11/05 20:19:17 maxv Exp $ +# $NetBSD: bsd.sys.mk,v 1.296 2019/11/15 09:44:44 maxv Exp $ # # Build definitions used for NetBSD source tree builds. @@ -247,7 +247,7 @@ CFLAGS+= ${KLEAKFLAGS.${.IMPSRC:T}:U${KL .if ${KCOV:U0} > 0 KCOVFLAGS= -fsanitize-coverage=trace-pc .for f in subr_kcov.c subr_lwp_specificdata.c subr_specificdata.c subr_asan.c \ - subr_csan.c + subr_csan.c subr_msan.c KCOVFLAGS.${f}= # empty .endfor CFLAGS+= ${KCOVFLAGS.${.IMPSRC:T}:U${KCOVFLAGS}} Index: src/sys/kern/subr_kcov.c diff -u src/sys/kern/subr_kcov.c:1.8 src/sys/kern/subr_kcov.c:1.9 --- src/sys/kern/subr_kcov.c:1.8 Sun May 26 05:41:45 2019 +++ src/sys/kern/subr_kcov.c Fri Nov 15 09:44:44 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: subr_kcov.c,v 1.8 2019/05/26 05:41:45 kamil Exp $ */ +/* $NetBSD: subr_kcov.c,v 1.9 2019/11/15 09:44:44 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -339,7 +339,7 @@ out: return error; } -static inline bool +static inline bool __nomsan in_interrupt(void) { return curcpu()->ci_idepth >= 0; @@ -347,7 +347,7 @@ in_interrupt(void) void __sanitizer_cov_trace_pc(void); -void +void __nomsan __sanitizer_cov_trace_pc(void) { extern int cold; @@ -388,7 +388,7 @@ __sanitizer_cov_trace_pc(void) } } -static void +static void __nomsan trace_cmp(uint64_t type, uint64_t arg1, uint64_t arg2, intptr_t pc) { extern int cold; @@ -433,7 +433,7 @@ trace_cmp(uint64_t type, uint64_t arg1, void __sanitizer_cov_trace_cmp1(uint8_t arg1, uint8_t arg2); -void +void __nomsan __sanitizer_cov_trace_cmp1(uint8_t arg1, uint8_t arg2) { @@ -443,7 +443,7 @@ __sanitizer_cov_trace_cmp1(uint8_t arg1, void __sanitizer_cov_trace_cmp2(uint16_t arg1, uint16_t arg2); -void +void __nomsan __sanitizer_cov_trace_cmp2(uint16_t arg1, uint16_t arg2) { @@ -453,7 +453,7 @@ __sanitizer_cov_trace_cmp2(uint16_t arg1 void __sanitizer_cov_trace_cmp4(uint32_t arg1, uint32_t arg2); -void +void __nomsan __sanitizer_cov_trace_cmp4(uint32_t arg1, uint32_t arg2) { @@ -463,7 +463,7 @@ __sanitizer_cov_trace_cmp4(uint32_t arg1 void __sanitizer_cov_trace_cmp8(uint64_t arg1, uint64_t arg2); -void +void __nomsan __sanitizer_cov_trace_cmp8(uint64_t arg1, uint64_t arg2) { @@ -473,7 +473,7 @@ __sanitizer_cov_trace_cmp8(uint64_t arg1 void __sanitizer_cov_trace_const_cmp1(uint8_t arg1, uint8_t arg2); -void +void __nomsan __sanitizer_cov_trace_const_cmp1(uint8_t arg1, uint8_t arg2) { @@ -483,7 +483,7 @@ __sanitizer_cov_trace_const_cmp1(uint8_t void __sanitizer_cov_trace_const_cmp2(uint16_t arg1, uint16_t arg2); -void +void __nomsan __sanitizer_cov_trace_const_cmp2(uint16_t arg1, uint16_t arg2) { @@ -493,7 +493,7 @@ __sanitizer_cov_trace_const_cmp2(uint16_ void __sanitizer_cov_trace_const_cmp4(uint32_t arg1, uint32_t arg2); -void +void __nomsan __sanitizer_cov_trace_const_cmp4(uint32_t arg1, uint32_t arg2) { @@ -503,7 +503,7 @@ __sanitizer_cov_trace_const_cmp4(uint32_ void __sanitizer_cov_trace_const_cmp8(uint64_t arg1, uint64_t arg2); -void +void __nomsan __sanitizer_cov_trace_const_cmp8(uint64_t arg1, uint64_t arg2) { @@ -513,7 +513,7 @@ __sanitizer_cov_trace_const_cmp8(uint64_ void __sanitizer_cov_trace_switch(uint64_t val, uint64_t *cases); -void +void __nomsan __sanitizer_cov_trace_switch(uint64_t val, uint64_t *cases) { uint64_t i, nbits, ncases, type;
CVS commit: src
Module Name:src Committed By: maxv Date: Fri Nov 15 09:44:44 UTC 2019 Modified Files: src/share/mk: bsd.sys.mk src/sys/kern: subr_kcov.c Log Message: Make kMSan compatible with KCOV. With kMSan we are forced to stay with the fsanitize flag on subr_kcov.c, which means that kMSan will instrument KCOV. We add a bunch of __nomsan attributes to reduce this instrumentation, but it does not remove it completely. That's fine. To generate a diff of this commit: cvs rdiff -u -r1.295 -r1.296 src/share/mk/bsd.sys.mk cvs rdiff -u -r1.8 -r1.9 src/sys/kern/subr_kcov.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/arch
Module Name:src Committed By: maxv Date: Fri Nov 15 09:03:26 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/i386/i386: cpufunc.S src/sys/arch/x86/include: pio.h Log Message: Remove the ins* and outs* functions. Not sanitizer-friendly, and unused anyway. To generate a diff of this commit: cvs rdiff -u -r1.47 -r1.48 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.36 -r1.37 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.9 -r1.10 src/sys/arch/x86/include/pio.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/amd64/amd64/cpufunc.S diff -u src/sys/arch/amd64/amd64/cpufunc.S:1.47 src/sys/arch/amd64/amd64/cpufunc.S:1.48 --- src/sys/arch/amd64/amd64/cpufunc.S:1.47 Thu Nov 14 16:23:52 2019 +++ src/sys/arch/amd64/amd64/cpufunc.S Fri Nov 15 09:03:26 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cpufunc.S,v 1.47 2019/11/14 16:23:52 maxv Exp $ */ +/* $NetBSD: cpufunc.S,v 1.48 2019/11/15 09:03:26 maxv Exp $ */ /* * Copyright (c) 1998, 2007, 2008 The NetBSD Foundation, Inc. @@ -349,15 +349,6 @@ ENTRY(inb) ret END(inb) -ENTRY(insb) - movl %edx, %ecx - movl %edi, %edx - movq %rsi, %rdi - rep - insb - ret -END(insb) - ENTRY(inw) movq %rdi, %rdx xorq %rax, %rax @@ -366,15 +357,6 @@ ENTRY(inw) ret END(inw) -ENTRY(insw) - movl %edx, %ecx - movl %edi, %edx - movq %rsi, %rdi - rep - insw - ret -END(insw) - ENTRY(inl) movq %rdi, %rdx xorq %rax, %rax @@ -383,15 +365,6 @@ ENTRY(inl) ret END(inl) -ENTRY(insl) - movl %edx, %ecx - movl %edi, %edx - movq %rsi, %rdi - rep - insl - ret -END(insl) - ENTRY(outb) movq %rdi, %rdx movq %rsi, %rax @@ -399,14 +372,6 @@ ENTRY(outb) ret END(outb) -ENTRY(outsb) - movl %edx, %ecx - movl %edi, %edx - rep - outsb - ret -END(outsb) - ENTRY(outw) movq %rdi, %rdx movq %rsi, %rax @@ -414,25 +379,9 @@ ENTRY(outw) ret END(outw) -ENTRY(outsw) - movl %edx, %ecx - movl %edi, %edx - rep - outsw - ret -END(outsw) - ENTRY(outl) movq %rdi, %rdx movq %rsi, %rax outl %eax, %dx ret END(outl) - -ENTRY(outsl) - movl %edx, %ecx - movl %edi, %edx - rep - outsl - ret -END(outsl) Index: src/sys/arch/i386/i386/cpufunc.S diff -u src/sys/arch/i386/i386/cpufunc.S:1.36 src/sys/arch/i386/i386/cpufunc.S:1.37 --- src/sys/arch/i386/i386/cpufunc.S:1.36 Wed Oct 30 17:06:57 2019 +++ src/sys/arch/i386/i386/cpufunc.S Fri Nov 15 09:03:26 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cpufunc.S,v 1.36 2019/10/30 17:06:57 maxv Exp $ */ +/* $NetBSD: cpufunc.S,v 1.37 2019/11/15 09:03:26 maxv Exp $ */ /*- * Copyright (c) 1998, 2007 The NetBSD Foundation, Inc. @@ -38,7 +38,7 @@ #include #include -__KERNEL_RCSID(0, "$NetBSD: cpufunc.S,v 1.36 2019/10/30 17:06:57 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cpufunc.S,v 1.37 2019/11/15 09:03:26 maxv Exp $"); #include "opt_xen.h" @@ -247,17 +247,6 @@ ENTRY(inb) ret END(inb) -ENTRY(insb) - pushl %edi - movl 8(%esp), %edx - movl 12(%esp), %edi - movl 16(%esp), %ecx - rep - insb - popl %edi - ret -END(insb) - ENTRY(inw) movl 4(%esp), %edx xorl %eax, %eax @@ -265,34 +254,12 @@ ENTRY(inw) ret END(inw) -ENTRY(insw) - pushl %edi - movl 8(%esp), %edx - movl 12(%esp), %edi - movl 16(%esp), %ecx - rep - insw - popl %edi - ret -END(insw) - ENTRY(inl) movl 4(%esp), %edx inl %dx, %eax ret END(inl) -ENTRY(insl) - pushl %edi - movl 8(%esp), %edx - movl 12(%esp), %edi - movl 16(%esp), %ecx - rep - insl - popl %edi - ret -END(insl) - ENTRY(outb) movl 4(%esp), %edx movl 8(%esp), %eax @@ -300,17 +267,6 @@ ENTRY(outb) ret END(outb) -ENTRY(outsb) - pushl %esi - movl 8(%esp), %edx - movl 12(%esp), %esi - movl 16(%esp), %ecx - rep - outsb - popl %esi - ret -END(outsb) - ENTRY(outw) movl 4(%esp), %edx movl 8(%esp), %eax @@ -318,31 +274,9 @@ ENTRY(outw) ret END(outw) -ENTRY(outsw) - pushl %esi - movl 8(%esp), %edx - movl 12(%esp), %esi - movl 16(%esp), %ecx - rep - outsw - popl %esi - ret -END(outsw) - ENTRY(outl) movl 4(%esp), %edx movl 8(%esp), %eax outl %eax, %dx ret END(outl) - -ENTRY(outsl) - pushl %esi - movl 8(%esp), %edx - movl 12(%esp), %esi - movl 16(%esp), %ecx - rep - outsl - popl %esi - ret -END(outsl) Index: src/sys/arch/x86/include/pio.h diff -u src/sys/arch/x86/include/pio.h:1.9 src/sys/arch/x86/include/pio.h:1.10 --- src/sys/arch/x86/include/pio.h:1.9 Sun May 22 16:01:43 2011 +++ src/sys/arch/x86/include/pio.h Fri Nov 15 09:03:26 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: pio.h,v 1.9 2011/05/22 16:01:43 christos Exp $ */ +/* $NetBSD: pio.h,v 1.10 2019/11/15 09:03:26 maxv Exp $ */ /*- * Copyright (c) 1998 The NetBSD Foundation, Inc. @@ -37,17 +37,11 @@ */ uint8_t inb(unsigned); -void insb(unsigned, void *, int); uint16_t inw(unsigned); -void insw(unsigned, void *, int); uint32_t inl(unsigned); -void insl(unsigned, void *, int); void outb(unsigned, uint8_t); -void outsb(unsigned, void *, int); void outw(unsigned,
CVS commit: src/sys/arch
Module Name:src Committed By: maxv Date: Fri Nov 15 09:03:26 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/i386/i386: cpufunc.S src/sys/arch/x86/include: pio.h Log Message: Remove the ins* and outs* functions. Not sanitizer-friendly, and unused anyway. To generate a diff of this commit: cvs rdiff -u -r1.47 -r1.48 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.36 -r1.37 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.9 -r1.10 src/sys/arch/x86/include/pio.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 15 08:11:37 UTC 2019 Modified Files: src/sys/kern: subr_csan.c src/sys/sys: systm.h Log Message: Instrument copyout() in kCSan, for parity with kMSan. To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/sys/kern/subr_csan.c cvs rdiff -u -r1.289 -r1.290 src/sys/sys/systm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Fri Nov 15 08:11:37 UTC 2019 Modified Files: src/sys/kern: subr_csan.c src/sys/sys: systm.h Log Message: Instrument copyout() in kCSan, for parity with kMSan. To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/sys/kern/subr_csan.c cvs rdiff -u -r1.289 -r1.290 src/sys/sys/systm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/subr_csan.c diff -u src/sys/kern/subr_csan.c:1.4 src/sys/kern/subr_csan.c:1.5 --- src/sys/kern/subr_csan.c:1.4 Thu Nov 14 16:56:13 2019 +++ src/sys/kern/subr_csan.c Fri Nov 15 08:11:37 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: subr_csan.c,v 1.4 2019/11/14 16:56:13 maxv Exp $ */ +/* $NetBSD: subr_csan.c,v 1.5 2019/11/15 08:11:37 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: subr_csan.c,v 1.4 2019/11/14 16:56:13 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: subr_csan.c,v 1.5 2019/11/15 08:11:37 maxv Exp $"); #include #include @@ -326,17 +326,20 @@ kcsan_strlen(const char *str) #undef copyinstr #undef copyoutstr #undef copyin +#undef copyout int kcsan_kcopy(const void *, void *, size_t); int kcsan_copystr(const void *, void *, size_t, size_t *); int kcsan_copyinstr(const void *, void *, size_t, size_t *); int kcsan_copyoutstr(const void *, void *, size_t, size_t *); int kcsan_copyin(const void *, void *, size_t); +int kcsan_copyout(const void *, void *, size_t); int kcopy(const void *, void *, size_t); int copystr(const void *, void *, size_t, size_t *); int copyinstr(const void *, void *, size_t, size_t *); int copyoutstr(const void *, void *, size_t, size_t *); int copyin(const void *, void *, size_t); +int copyout(const void *, void *, size_t); int kcsan_kcopy(const void *src, void *dst, size_t len) @@ -361,6 +364,13 @@ kcsan_copyin(const void *uaddr, void *ka } int +kcsan_copyout(const void *kaddr, void *uaddr, size_t len) +{ + kcsan_access((uintptr_t)kaddr, len, false, false, __RET_ADDR); + return copyout(kaddr, uaddr, len); +} + +int kcsan_copyinstr(const void *uaddr, void *kaddr, size_t len, size_t *done) { kcsan_access((uintptr_t)kaddr, len, true, false, __RET_ADDR); Index: src/sys/sys/systm.h diff -u src/sys/sys/systm.h:1.289 src/sys/sys/systm.h:1.290 --- src/sys/sys/systm.h:1.289 Thu Nov 14 16:23:53 2019 +++ src/sys/sys/systm.h Fri Nov 15 08:11:36 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: systm.h,v 1.289 2019/11/14 16:23:53 maxv Exp $ */ +/* $NetBSD: systm.h,v 1.290 2019/11/15 08:11:36 maxv Exp $ */ /*- * Copyright (c) 1982, 1988, 1991, 1993 @@ -299,11 +299,12 @@ int kcsan_copystr(const void *, void *, int kcsan_copyinstr(const void *, void *, size_t, size_t *); int kcsan_copyoutstr(const void *, void *, size_t, size_t *); int kcsan_copyin(const void *, void *, size_t); -int copyout(const void *, void *, size_t); +int kcsan_copyout(const void *, void *, size_t); #define copystr kcsan_copystr #define copyinstr kcsan_copyinstr #define copyoutstr kcsan_copyoutstr #define copyin kcsan_copyin +#define copyout kcsan_copyout #elif defined(_KERNEL) && defined(KMSAN) int kmsan_copystr(const void *, void *, size_t, size_t *); int kmsan_copyinstr(const void *, void *, size_t, size_t *);
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Thu Nov 14 17:09:23 UTC 2019 Modified Files: src/sys/arch/aarch64/aarch64: aarch64_machdep.c pmap.c src/sys/arch/amd64/amd64: machdep.c src/sys/arch/x86/x86: pmap.c src/sys/sys: asan.h Log Message: Mark several kASan functions with __nothing, to avoid annoying #ifdefs. Same as kCSan and kMSan. To generate a diff of this commit: cvs rdiff -u -r1.33 -r1.34 src/sys/arch/aarch64/aarch64/aarch64_machdep.c cvs rdiff -u -r1.49 -r1.50 src/sys/arch/aarch64/aarch64/pmap.c cvs rdiff -u -r1.340 -r1.341 src/sys/arch/amd64/amd64/machdep.c cvs rdiff -u -r1.339 -r1.340 src/sys/arch/x86/x86/pmap.c cvs rdiff -u -r1.11 -r1.12 src/sys/sys/asan.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Thu Nov 14 17:09:23 UTC 2019 Modified Files: src/sys/arch/aarch64/aarch64: aarch64_machdep.c pmap.c src/sys/arch/amd64/amd64: machdep.c src/sys/arch/x86/x86: pmap.c src/sys/sys: asan.h Log Message: Mark several kASan functions with __nothing, to avoid annoying #ifdefs. Same as kCSan and kMSan. To generate a diff of this commit: cvs rdiff -u -r1.33 -r1.34 src/sys/arch/aarch64/aarch64/aarch64_machdep.c cvs rdiff -u -r1.49 -r1.50 src/sys/arch/aarch64/aarch64/pmap.c cvs rdiff -u -r1.340 -r1.341 src/sys/arch/amd64/amd64/machdep.c cvs rdiff -u -r1.339 -r1.340 src/sys/arch/x86/x86/pmap.c cvs rdiff -u -r1.11 -r1.12 src/sys/sys/asan.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/arch/aarch64/aarch64/aarch64_machdep.c diff -u src/sys/arch/aarch64/aarch64/aarch64_machdep.c:1.33 src/sys/arch/aarch64/aarch64/aarch64_machdep.c:1.34 --- src/sys/arch/aarch64/aarch64/aarch64_machdep.c:1.33 Thu Nov 14 16:48:51 2019 +++ src/sys/arch/aarch64/aarch64/aarch64_machdep.c Thu Nov 14 17:09:22 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: aarch64_machdep.c,v 1.33 2019/11/14 16:48:51 maxv Exp $ */ +/* $NetBSD: aarch64_machdep.c,v 1.34 2019/11/14 17:09:22 maxv Exp $ */ /*- * Copyright (c) 2014 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(1, "$NetBSD: aarch64_machdep.c,v 1.33 2019/11/14 16:48:51 maxv Exp $"); +__KERNEL_RCSID(1, "$NetBSD: aarch64_machdep.c,v 1.34 2019/11/14 17:09:22 maxv Exp $"); #include "opt_arm_debug.h" #include "opt_ddb.h" @@ -434,9 +434,7 @@ initarm_common(vaddr_t kvm_base, vsize_t */ pmap_bootstrap(kernelvmstart, VM_MAX_KERNEL_ADDRESS); -#ifdef KASAN kasan_init(); -#endif /* * setup lwp0 Index: src/sys/arch/aarch64/aarch64/pmap.c diff -u src/sys/arch/aarch64/aarch64/pmap.c:1.49 src/sys/arch/aarch64/aarch64/pmap.c:1.50 --- src/sys/arch/aarch64/aarch64/pmap.c:1.49 Thu Nov 14 16:48:51 2019 +++ src/sys/arch/aarch64/aarch64/pmap.c Thu Nov 14 17:09:22 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: pmap.c,v 1.49 2019/11/14 16:48:51 maxv Exp $ */ +/* $NetBSD: pmap.c,v 1.50 2019/11/14 17:09:22 maxv Exp $ */ /* * Copyright (c) 2017 Ryo Shimizu @@ -27,7 +27,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.49 2019/11/14 16:48:51 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.50 2019/11/14 17:09:22 maxv Exp $"); #include "opt_arm_debug.h" #include "opt_ddb.h" @@ -1681,11 +1681,9 @@ _pmap_enter(struct pmap *pm, vaddr_t va, opte = atomic_swap_64(ptep, 0); need_sync_icache = (prot & VM_PROT_EXECUTE); -#ifdef KASAN if (!user) { kasan_shadow_map((void *)va, PAGE_SIZE); } -#endif /* for lock ordering for pg and opg */ pgs[0] = pg; Index: src/sys/arch/amd64/amd64/machdep.c diff -u src/sys/arch/amd64/amd64/machdep.c:1.340 src/sys/arch/amd64/amd64/machdep.c:1.341 --- src/sys/arch/amd64/amd64/machdep.c:1.340 Thu Nov 14 16:48:51 2019 +++ src/sys/arch/amd64/amd64/machdep.c Thu Nov 14 17:09:23 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: machdep.c,v 1.340 2019/11/14 16:48:51 maxv Exp $ */ +/* $NetBSD: machdep.c,v 1.341 2019/11/14 17:09:23 maxv Exp $ */ /* * Copyright (c) 1996, 1997, 1998, 2000, 2006, 2007, 2008, 2011 @@ -110,7 +110,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: machdep.c,v 1.340 2019/11/14 16:48:51 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: machdep.c,v 1.341 2019/11/14 17:09:23 maxv Exp $"); #include "opt_modular.h" #include "opt_user_ldt.h" @@ -1684,9 +1684,7 @@ init_x86_64(paddr_t first_avail) init_pte(); -#ifdef KASAN kasan_early_init((void *)lwp0uarea); -#endif uvm_lwp_setuarea(, lwp0uarea); @@ -1766,9 +1764,7 @@ init_x86_64(paddr_t first_avail) init_x86_msgbuf(); -#ifdef KASAN kasan_init(); -#endif kcsan_init(); kmsan_init((void *)lwp0uarea); Index: src/sys/arch/x86/x86/pmap.c diff -u src/sys/arch/x86/x86/pmap.c:1.339 src/sys/arch/x86/x86/pmap.c:1.340 --- src/sys/arch/x86/x86/pmap.c:1.339 Thu Nov 14 16:23:52 2019 +++ src/sys/arch/x86/x86/pmap.c Thu Nov 14 17:09:23 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: pmap.c,v 1.339 2019/11/14 16:23:52 maxv Exp $ */ +/* $NetBSD: pmap.c,v 1.340 2019/11/14 17:09:23 maxv Exp $ */ /* * Copyright (c) 2008, 2010, 2016, 2017 The NetBSD Foundation, Inc. @@ -130,14 +130,13 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.339 2019/11/14 16:23:52 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.340 2019/11/14 17:09:23 maxv Exp $"); #include "opt_user_ldt.h" #include "opt_lockdebug.h" #include "opt_multiprocessor.h" #include "opt_xen.h" #include "opt_svs.h" -#include "opt_kasan.h" #include "opt_kaslr.h" #include @@ -4573,11 +4572,8 @@ pmap_growkernel(vaddr_t maxkvaddr) } #endif -#ifdef KASAN kasan_shadow_map((void *)pmap_maxkvaddr, (size_t)(maxkvaddr - pmap_maxkvaddr)); -#endif - kmsan_shadow_map((void *)pmap_maxkvaddr, (size_t)(maxkvaddr
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Thu Nov 14 16:56:13 UTC 2019 Modified Files: src/sys/kern: subr_csan.c Log Message: Don't include "opt_kcsan.h" since there's already included. To generate a diff of this commit: cvs rdiff -u -r1.3 -r1.4 src/sys/kern/subr_csan.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/kern/subr_csan.c diff -u src/sys/kern/subr_csan.c:1.3 src/sys/kern/subr_csan.c:1.4 --- src/sys/kern/subr_csan.c:1.3 Fri Nov 8 12:36:10 2019 +++ src/sys/kern/subr_csan.c Thu Nov 14 16:56:13 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: subr_csan.c,v 1.3 2019/11/08 12:36:10 maxv Exp $ */ +/* $NetBSD: subr_csan.c,v 1.4 2019/11/14 16:56:13 maxv Exp $ */ /* * Copyright (c) 2019 The NetBSD Foundation, Inc. @@ -30,9 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: subr_csan.c,v 1.3 2019/11/08 12:36:10 maxv Exp $"); - -#include "opt_kcsan.h" +__KERNEL_RCSID(0, "$NetBSD: subr_csan.c,v 1.4 2019/11/14 16:56:13 maxv Exp $"); #include #include
CVS commit: src/sys/kern
Module Name:src Committed By: maxv Date: Thu Nov 14 16:56:13 UTC 2019 Modified Files: src/sys/kern: subr_csan.c Log Message: Don't include "opt_kcsan.h" since there's already included. To generate a diff of this commit: cvs rdiff -u -r1.3 -r1.4 src/sys/kern/subr_csan.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys
Module Name:src Committed By: maxv Date: Thu Nov 14 16:48:51 UTC 2019 Modified Files: src/sys/arch/aarch64/aarch64: aarch64_machdep.c pmap.c src/sys/arch/amd64/amd64: machdep.c src/sys/uvm: uvm_glue.c Log Message: Don't include "opt_kasan.h" when there's already included. To generate a diff of this commit: cvs rdiff -u -r1.32 -r1.33 src/sys/arch/aarch64/aarch64/aarch64_machdep.c cvs rdiff -u -r1.48 -r1.49 src/sys/arch/aarch64/aarch64/pmap.c cvs rdiff -u -r1.339 -r1.340 src/sys/arch/amd64/amd64/machdep.c cvs rdiff -u -r1.168 -r1.169 src/sys/uvm/uvm_glue.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/doc
Module Name:src Committed By: maxv Date: Thu Nov 14 16:27:26 UTC 2019 Modified Files: src/doc: CHANGES Log Message: Note kMSan. To generate a diff of this commit: cvs rdiff -u -r1.2613 -r1.2614 src/doc/CHANGES Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/doc/CHANGES diff -u src/doc/CHANGES:1.2613 src/doc/CHANGES:1.2614 --- src/doc/CHANGES:1.2613 Wed Nov 13 10:52:40 2019 +++ src/doc/CHANGES Thu Nov 14 16:27:26 2019 @@ -1,4 +1,4 @@ -# LIST OF CHANGES FROM LAST RELEASE: <$Revision: 1.2613 $> +# LIST OF CHANGES FROM LAST RELEASE: <$Revision: 1.2614 $> # # # [Note: This file does not mention every change made to the NetBSD source tree. @@ -76,3 +76,5 @@ Changes from NetBSD 9.0 to NetBSD 10.0: [msaitoh 20191107] tmux(1): Imported 2.9a. [christos 20191112] dhcpcd(8): Import 8.1.2. [roy 20191113] + amd64: Add support for kMSan - Kernel Memory Sanitizer. + [maxv 20191114]
CVS commit: src/doc
Module Name:src Committed By: maxv Date: Thu Nov 14 16:27:26 UTC 2019 Modified Files: src/doc: CHANGES Log Message: Note kMSan. To generate a diff of this commit: cvs rdiff -u -r1.2613 -r1.2614 src/doc/CHANGES Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.