tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: [PATCH v3 2/2] Implement PT_GETXSTATE and PT_SETXSTATE



Le 19/06/2019 à 14:20, Michał Górny a écrit :> On Wed, 2019-06-19 at 06:32 +0200, Maxime Villard wrote:
Can you provide a unified patch so I can review quickly? Sorry for the delay

Here you are.  Note that the manpage is not updated yet, as I'm waiting
for your comments on the xs_rfbm approach.

+		memcpy(&(xstate -> field),					\
+				(char*)fpu_save + x86_xsave_offsets[xsave_val],	\
+				sizeof(xstate -> field));			\

Please use proper KNF here and in other places

+		fpu_save->sv_xsave_hdr.xsh_xstate_bv =
+			(~xstate->xs_rfbm | xstate->xs_xstate_bv) &
+			(xstate->xs_rfbm | fpu_save->sv_xsave_hdr.xsh_xstate_bv);

Can't we just do this? Seems simpler

	fpu_save->sv_xsave_hdr.xsh_xstate_bv =
	    (fpu_save->sv_xsave_hdr.xsh_xstate_bv & ~xstate->xs_rfbm) |
	    xstate->xs_xstate_bv;

Unfortunately, in this code you will likely hit a race; between the moment you
set fpu_save->sv_xsave_hdr.xsh_xstate_bv and the moment you reach COPY_COMPONENT,
the thread could have been context-switched, and xstate_bv could have been
clobbered.

I think you can work around this race by adding kpreempt_disable/enable at
the beginning/end of the function. ISTR there is the same problem in the
other fpregs ptrace functions. In fact, there are a lot of problems in our
FPU code right now.

Otherwise the rfbm approach seems good to me.


Home | Main Index | Thread Index | Old Index