[PATCH] Fix {get, set}rlimit decoding with unreliable SIZEOF_RLIM_T

[James Hogan]

Hi Dmitry,

On 14/05/14 15:17, Dmitry V. Levin wrote:
> On Fri, May 02, 2014 at 02:15:41PM +0100, James Hogan wrote:
>> When strace is built with large file support definitions in CFLAGS (as
>> may be provided by buildroot) the C library headers may expose a 64-bit
>> rlim_t even though the struct rlimit fields used by the system call
>> interface are only 32-bit. The SIZEOF_RLIM_T will then be 8 which
>> results in bad decoding of the getrlimit and setrlimit syscalls.
> 
> If SIZEOF_RLIM_T cannot be relied upon, it shouldn't be used, and its
> definition has to be removed as well.

True.

>> This is fixed by removing the "#if SIZEOF_RLIM_T == 4 ||
>> SUPPORTED_PERSONALITIES > 1" conditional, since the remaining code
>> already handles multiple personalities based on the value of
>> current_wordsize, which is set correctly even for a single personality.
> 
> I have two questions wrt this patch:
> 
> 1. Removing the conditional would result with print_rlimit32 defined for
> all architectures, including pure 64bit ones.  I suppose we need a check
> like this anyway:
> 
> #if PERSONALITY0_WORDSIZE == 4 || \
>     (SUPPORTED_PERSONALITIES > 1 && PERSONALITY1_WORDSIZE == 4) || \
>     (SUPPORTED_PERSONALITIES > 2 && PERSONALITY2_WORDSIZE == 4)

There don't appear to be any arches supported (according to defs.h) with
>2 personalities with all wordsizes the same.

Where there are 2 personalities with the same wordsize the definition of
current_wordsize takes this into account and defines it as
PERSONALITY0_WORDSIZE.

So this would seem to allow the compiler to optimise out whichever
print_rlimit{32,64} is unused, and a quick look at the x86_64 assembly
output (gcc 4.7.3 -O2) with a hacked current_wordsize in resource.c
confirms that.

There is of course the "!addr" and "!verbose(tcp || (exiting(tcp) &&
syserror(tcp))" conditions which seem to be skipped for the pure 64-bit
case at the moment which I'm not sure was intentional anyway?

> 2. I'm not sure about architectures where kernel wordsize is 8 but user
> wordsize is 4 (like x32 and mips n32); what's the correct rlim_t for these
> architectures?

Hmm, that's a good point.

MIPS n32 appears to use the default __kernel_ulong_t define of unsigned
long (32bit) so it would work fine I think (although sadly it appears
that strace doesn't support personalities on MIPS yet).

x32 appears to define __kernel_ulong_t as unsigned long long (64bit), so
strace handling of {get,set}rlimit would already appear to be broken for
that (this patch doesn't make that any worse).

Cheers
James

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140516/97aa2682/attachment.bin>