Re: [PATCH] audit: ia32entry.S drops useful return value sign bits
On Tue, 2011-05-24 at 09:18 -0700, H. Peter Anvin wrote:
On 05/24/2011 06:55 AM, Thomas Gleixner wrote:
>> This seems like the fundamental design error.
>
> I don't think so. We run in 64bit mode here and call into 64bit code
> which expects a long being 64bit and not a 32bit truncated value. The
> audit code is pure kernel stuff and this is not the return to
> userspace.
I don't agree, this is about auditing the return to userspace. For the
IA32 entry point, the return value is a 32-bit value even if we happen
to return to 64-bit userspace. Treating it as anything else is asking
for a security hole when we audit something that isn't what we do.
As such, the right thing to do is probably:
movl %eax, %esi
cmpl $-MAX_ERRNO, %eax
jb 1f
movslq %eax, %rsi
1: setae %al
I'll do it that way if you want. But you now have an extra jb and an
extra movl, neither of which do anything at all. It's no different than
movq %rax, %rsi
cmp{q,l} $-MAX_ERRNO, %{r,e}ax
setae %al
I know it's the same because I spent forever trying to hunt down movslq.
I don't understand why it's not in the Intel® 64 and IA-32 Architectures
Software Developer’s Manual Volume 2 (2A & 2B): Instruction Set
Reference, A-Z. That's exactly what I talked about, truncating the
upper 32 bits just the sign extend them right back.
I guess it comes down to picking one of these 3:
My version:
movq %rax,%rsi /* second arg, syscall return value */
cmpl $-MAX_ERRNO,%rax /* is it < 0? */
setbe %al /* 1 if so, 0 if not */
movzbl %al,%edi /* zero-extend that into %edi */
call __audit_syscall_exit
VS hpa version:
movl %eax,%esi /* move 32bits to second arg */
cmpl $-MAX_ERRNO,%eax /* check if fail */
jbe 1f
movslq %eax, %rsi /* re-sign-extend eax */
1: setbe %al
movzbl %al,%edi
call __audit_syscall_exit
VS alternate of hpa version without set:
movl $1,%edi /* syscall success argument */
movl %eax,%esi /* move 32bits to second arg */
cmpl $-MAX_ERRNO,%eax /* check if fail */
jbe 1f
xor %edi,%edi /* syscall failure argument */
movslq %eax, %rsi /* resign-extend eax */
1: call __audit_syscall_exit
If I have to go with the hpa version of truncation followed by sign
extension, is it any better/cheap/faster to use just movl in the
'common' case and movl+xor in the uncommon syscall failure? I don't
know how expensive or large the set+movzbl is....