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

[Dmitry V. Levin]

On Tue, May 20, 2014 at 11:24:51PM +0100, James Hogan wrote:
> Hi Dmitry,
> 
> On 16/05/14 17:02, Dmitry V. Levin wrote:
> > On Fri, May 16, 2014 at 12:53:29PM +0100, James Hogan wrote:
> >> On 14/05/14 15:17, Dmitry V. Levin wrote:
> > [...]
> >>> 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.
> > 
> > Yes, so the check could be simplified to
> > #if !defined(current_wordsize) || current_wordsize == 4
> 
> Unfortunately this confuses preprocessor when using this definition:
> # define PERSONALITY0_WORDSIZE (int)(sizeof(long))

Thanks, I missed this case.

> Maybe with this extra hunk?
> 
> diff --git a/defs.h b/defs.h
> index 074c8f0..4e06a92 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -370,7 +370,7 @@ struct arm_pt_regs {
>  # define DEFAULT_PERSONALITY 0
>  #endif
>  #ifndef PERSONALITY0_WORDSIZE
> -# define PERSONALITY0_WORDSIZE (int)(sizeof(long))
> +# define PERSONALITY0_WORDSIZE SIZEOF_LONG
>  #endif
> 
>  #if defined(I386) || defined(X86_64)

Yes.

> >>> 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).
> > 
> > Thanks.  I suppose this patch fixes all these rlim_t issues:
> > 
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -309,7 +309,6 @@ AC_CACHE_CHECK([for BLKGETSIZE64], [ac_cv_have_blkgetsize64],
> >  AC_CHECK_SIZEOF([long])
> >  AC_CHECK_SIZEOF([long long])
> >  AC_CHECK_SIZEOF([off_t],,[#include <sys/types.h>])
> > -AC_CHECK_SIZEOF([rlim_t],,[#include <sys/resource.h>])
> >  
> >  AC_CACHE_CHECK([for SA_RESTORER], [st_cv_sa_restorer],
> >  	       [st_cv_sa_restorer="$(echo SA_RESTORER |
> > --- a/resource.c
> > +++ b/resource.c
> > @@ -88,10 +88,6 @@ static const struct xlat resources[] = {
> >  	XLAT_END
> >  };
> >  
> > -#if !(SIZEOF_RLIM_T == 4 || SIZEOF_RLIM_T == 8)
> > -# error "Unsupported SIZEOF_RLIM_T value"
> > -#endif
> > -
> >  static const char *
> >  sprint_rlim64(uint64_t lim)
> >  {
> > @@ -135,7 +131,7 @@ decode_rlimit64(struct tcb *tcp, unsigned long addr)
> >  		print_rlimit64(tcp, addr);
> >  }
> >  
> > -#if SIZEOF_RLIM_T == 4 || SUPPORTED_PERSONALITIES > 1
> > +#if !defined(current_wordsize) || current_wordsize == 4
> >  
> >  static const char *
> >  sprint_rlim32(uint32_t lim)
> > @@ -176,22 +172,22 @@ decode_rlimit(struct tcb *tcp, unsigned long addr)
> >  	else if (!verbose(tcp) || (exiting(tcp) && syserror(tcp)))
> >  		tprintf("%#lx", addr);
> >  	else {
> > -# if SIZEOF_RLIM_T == 4
> > -		print_rlimit32(tcp, addr);
> > +# ifdef X32
> 
> Does this need to take X86_64 into account too, since I presume
> personality 1 is X32 too?

Indeed, i386 is the only personality on X86_64 and X32 with 32-bit rlim_t.

I've pushed the fix with these two corrections applied.
Thanks!


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140521/f56e276a/attachment.bin>