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

[Eric Paris <eparis@redhat.com>]

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
https://www.redhat.com/mailman/listinfo/linux-audit