lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Sep 2007 15:59:33 -0400 From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca> To: Andi Kleen <andi@...stfloor.org> Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org, pageexec@...email.hu Subject: Re: [patch 05/10] Text Edit Lock - Alternative code for i386 and x86_64 * Andi Kleen (andi@...stfloor.org) wrote: > On Fri, Sep 07, 2007 at 10:04:42AM -0400, Mathieu Desnoyers wrote: > > * Andi Kleen (andi@...stfloor.org) wrote: > > > On Thu, Sep 06, 2007 at 04:01:29PM -0400, Mathieu Desnoyers wrote: > > > > + sync_core(); > > > > + /* Not strictly needed, but can speed CPU recovery up. */ > > > > > > That turned out to break on some VIA CPUs. Should be removed. > > > > > > > Hrm, when does it break ? At boot time ? Is it the cpuid that breaks or > > Yes. > > > the clflush ? How do you work around the problem when sync_core or > > The CLFLUSH > > > clflush is called from elsewhere; does it cause a problem if I call it > > when I update immediate values ? > > Unknown currently what are the exact circumstances. > > For the other cases it is ignored right now, but when we get > more information it might be needed to clear the CLFLUSH > feature bit on those CPUs. > Ok, it's better to simply remove the CLFLUSH since it's not needed anyway. Do you recommend to only remove the CLFLUSH from the _early code (executed before alternatives are applied) or to remove the CLFLUSH from the normal execution time code also ? > > Is it me or __inline_memcpy is simply a copy of i386's __memcpy ? > > Is there any reason for this name change ? > > x86-64 __memcpy does something different. > > It might make more sense > > At some point I hope to change the i386 setup to be more like x86-64 > anyways -- the x86-64 version is imho much better. > It looks like gorund work that should be done in i386 before we start using __inline_memcpy in alternative.c which is shared between i386 and x86_64. I haven't seen the alternative that would touch memcpy at all anyway, am I missing something ? Also, being "faster" is not really an issue there, since it is not done often. The only thing that matters is if memcpy could be touched by alternative.c. > > A- ugly > > B- breaking vim syntax highlighting. (actually, all the rest of the > > file becomes weird after that. The problem is similar to declaration > > of #defile name ({ some code }). It does not really matter as long as > > it is in a header, but at the middle of a C file it gets rather > > annoying). (it never though I would use vim as a coding style > > reference) ;) > > Then define a macro > > #define BREAKPOINTS(x) \ > ((unsigned char [x]){ [0 ... x] = BREAKPOINT_INSTRUCTIONS }) > > and use that > How about adding : #define INIT_ARRAY(type, val, len) ((type [len]){ [0 ... len] = (val) }) to kernel.h ? It would be more generic. > > And what is rather different between the 2 functions is when we want to > > fill multiple bytes with the same pattern (I fill the unused part of my > > immediate values bypass with 0x90 nops, but I agree that I could use > > add_nops if it was exported). > > > > Declaration of a variable length array on text_set's stack would break > > older compilers, so I don't think it is a neat solution neither. kmalloc > > All supported gccs support variable length arrays. > ok > > The idea is to mimic the local_irq_save/restore semantic, where the > > flags argument is passed without &. This is why I use a macro instead of > > an inline function > > Sounds like a bogus idea to me. > So you would prefer unsigned long cr0; kernel_wp_save(&cr0) .. kernel_wp_restore(cr0); to unsigned long cr0; kernel_wp_save(cr0) .. kernel_wp_restore(cr0); ? It seems odd to me, since everyone would expect flags save/restore to behave like local_irq_save/restore. Or I may misunderstand your point. > > The good effect of disabling interrupts is that it would make sure no > > interrupt handler will run with WP flag cleared on the CPU. However, it > > Yes that was my point. Not a very strong one admittedly. > I agree that most kernel_wp_save/restore users should disable interrupts before calling it (it would be a "good practice"), but I don't expect to encapsulate irq disabling in these macros, since it is not mandatory. Mathieu > -Andi -- Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists