Re: [PATCH] audit: ia32entry.S drops useful return value sign bits

Eric Paris
2011-05-24T19:13:44Z

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....

--
Linux-audit mailing list
Linux-audit@redhat.com