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: Sun, 18 Mar 2018 03:49:24 +0000 From: Jason Vas Dias <jason.vas.dias@...il.com> To: Andi Kleen <andi@...stfloor.org> Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org, x86@...nel.org, mingo@...nel.org, peterz@...radead.org Subject: Re: [PATCH v4.16-rc5 2/2] x86/vdso: VDSO should handle clock_gettime(CLOCK_MONOTONIC_RAW) without syscall On 18/03/2018, Jason Vas Dias <jason.vas.dias@...il.com> wrote: (should have CC'ed to list, sorry) > On 17/03/2018, Andi Kleen <andi@...stfloor.org> wrote: >> >> That's quite a mischaracterization of the issue. gcc works as intended, >> but the kernel did not correctly supply a indirect call retpoline thunk >> to the vdso, and it just happened to work by accident with the old >> vdso. >> >>> >>> The automated test builds should now succeed with this patch. >> >> How about just adding the thunk function to the vdso object instead of >> this cheap hack? >> >> The other option would be to build vdso with inline thunks. >> >> But just disabling is completely the wrong action. >> >> -Andi >> > > Aha! Thanks for the clarification , Andi! > > I will do so and resend the 2nd patch. > > But is everyone agreed we should accept any slowdown for the timer > functions ? I personally don't think it is a good idea, but I will > regenerate the patch with the thunk function and without > the Makefile change. > > Thanks & Best Regards, > Jason > I am wondering if it is not better to avoid the thunk being generated and remove the Makefile patch ? I know that changing the switch in __vdso_clock_gettime() like this avoids the thunk : switch(clock) { case CLOCK_MONOTONIC: if (do_monotonic(ts) == VCLOCK_NONE) goto fallback; break; default: switch (clock) { case CLOCK_REALTIME: if (do_realtime(ts) == VCLOCK_NONE) goto fallback; break; case CLOCK_MONOTONIC_RAW: if (do_monotonic_raw(ts) == VCLOCK_NONE) goto fallback; break; case CLOCK_REALTIME_COARSE: do_realtime_coarse(ts); break; case CLOCK_MONOTONIC_COARSE: do_monotonic_coarse(ts); break; default: goto fallback; } return 0; fallback: ... } So at the cost of an unnecessary extra test of the clock parameter, the thunk is avoided . I wonder if the whole switch should be changed to an if / else clause ? Or, I know this might be unorthodox, but might work : #define _CAT(V1,V2) V1##V2 #define GTOD_CLK_LABEL(CLK) _CAT(_VCG_L_,CLK) #define MAX_CLK 16 //^^ ?? __vdso_clock_gettime( ... ) { ... static const void *clklbl_tab[MAX_CLK] ={ [ CLOCK_MONOTONIC ] = &>OD_CLK_LABEL(CLOCK_MONOTONIC) , [ CLOCK_MONOTONIC_RAW ] = &>OD_CLK_LABEL(CLOCK_MONOTONIC_RAW) , // and similarly for all clocks handled ... }; goto clklbl_tab[ clock & 0xf ] ; GTOD_CLK_LABEL(CLOCK_MONOTONIC) : if ( do_monotonic(ts) == VCLOCK_NONE ) goto fallback ; GTOD_CLK_LABEL(CLOCK_MONOTONIC_RAW) : if ( do_monotonic_raw(ts) == VCLOCK_NONE ) goto fallback ; ... // similarly for all clocks fallback: return vdso_fallback_gettime(clock,ts); } If a restructuring like that might be acceptable (with correct tab-based formatting) , and the VDSO can have such a table in its .BSS , I think it would avoid the thunk, and have the advantage of precomputing the jump table at compile-time, and would not require any indirect branches, I think. Any thoughts ? Thanks & Best regards, Jason ; G
Powered by blists - more mailing lists