2007-08-13 12:36:49

by Keith Owens

[permalink] [raw]
Subject: Unbalanced stack usage in arch/i386/math-emu/wm_sqrt.S

Originally sent to the maintainer of the i386 math-emu code
([email protected]) but that mail was bounced[1]. Is anybody
maintaining the math-emu code and do we even care about it anymore?

I am doing static code analysis on the kernel and have found a stack
imbalance in arch/i386/math-emu/wm_sqrt.S. 2.6.23-rc2, but it has
probably been there for a while.

The code starts off with

pushl %ebp
movl %esp,%ebp
#ifndef NON_REENTRANT_FPU
subl $28,%esp
#endif /* NON_REENTRANT_FPU */
pushl %esi
pushl %edi
pushl %ebx

At this point, the code is using 0x2c bytes of stack space.

.... do some work

sqrt_stage_2_finish:
sarl $1,%ecx /* divide by 2 */
rcrl $1,%eax

/* Form the new estimate in %esi:%edi */
movl %eax,%edi
addl %ecx,%esi

jnz sqrt_stage_2_done /* result should be [1..2) */

... still using 0x2c bytes of stack space

#ifdef PARANOID
/* It should be possible to get here only if the arg is ffff....ffff */
cmp $0xffffffff,FPU_fsqrt_arg_1
jnz sqrt_stage_2_error
#endif /* PARANOID */

/* The best rounded result. */
xorl %eax,%eax
decl %eax
movl %eax,%edi
movl %eax,%esi
movl $0x7fffffff,%eax
jmp sqrt_round_result

#ifdef PARANOID
sqrt_stage_2_error:

.... 0x2c bytes of stack space

pushl EX_INTERNAL|0x213

.... 0x30 bytes of stack space

call EXCEPTION

.... EXCEPTION is FPU_exception which only aborts if __DEBUG__ is
defined, __DEBUG__ is not defined. So FPU_exception will return
and we still have 0x30 bytes of stack used. But the code drops
through to sqrt_stage_2_done which (like the rest of the code) only
expects 0x2c bytes of stack ===> stack imbalance.

#endif /* PARANOID */

sqrt_stage_2_done:


The obvious fix is to add 'pop %eax' after 'call EXCEPTION' which will
remove the extra word from the stack. Alas that only fixes the stack
imbalance, but does it even make sense for the code to continue after
calling EXCEPTION?


[1] <[email protected]>: host suburbia.com.au[203.24.247.1] said: 554
<[email protected]>: Recipient address rejected: Access denied (in
reply to RCPT TO command)

The URL listed in MAINTAINERS for FPU Emulator gets a 404 as well.


2007-08-13 16:09:23

by Andi Kleen

[permalink] [raw]
Subject: Re: Unbalanced stack usage in arch/i386/math-emu/wm_sqrt.S

Keith Owens <[email protected]> writes:

> Originally sent to the maintainer of the i386 math-emu code
> ([email protected]) but that mail was bounced[1]. Is anybody
> maintaining the math-emu code and do we even care about it anymore?

I can take a patch, but I would prefer if it was tested
e.g. by force disabling the FPU and then running a program
to trigger that exception. If there is no test it
might be safer to keep the bug around than to run
untested code.

-Andi