[PATCH 1/3] util: change type of struct xlat.val to uint64_t

[Dmitry V. Levin]

On Wed, Mar 30, 2016 at 09:19:25PM -0400, Jeff Mahoney wrote:
> Some ioctls have flags fields that are 64-bit.  A 32-bit val means
> these flags will never be matched or printed.

struct xlat is used for various kinds of thing all over the code,
and this tiny change of type might have unforeseen consequences.

[...]
>  struct xlat {
> -	unsigned int val;
> +	uint64_t val;
>  	const char *str;
>  };
[...]
> -extern const char *xlookup(const struct xlat *, const unsigned int);
> -extern const char *xlat_search(const struct xlat *, const size_t, const unsigned int);
> +extern const char *xlookup(const struct xlat *, const uint64_t);
> +extern const char *xlat_search(const struct xlat *, const size_t, const uint64_t);
[...]
> -extern void printxvals(const unsigned int, const char *, const struct xlat *, ...);
> +extern void printxvals(const uint64_t, const char *, const struct xlat *, ...);
[...]
> -extern void addflags(const struct xlat *, int);
> -extern int printflags(const struct xlat *, int, const char *);
> -extern const char *sprintflags(const char *, const struct xlat *, int);
> +extern void addflags(const struct xlat *, uint64_t);
> +extern int printflags(const struct xlat *, uint64_t, const char *);
> +extern const char *sprintflags(const char *, const struct xlat *, uint64_t);

There is a potential sign extension bug in every place where a signed int
is passed to any function "upgraded" from int/unsigned int to uint64_t.

Have you checked all these places?

> @@ -132,16 +132,16 @@ xlookup(const struct xlat *xlat, const unsigned int val)
>  static int
>  xlat_bsearch_compare(const void *a, const void *b)
>  {
> -	const unsigned int val1 = (const unsigned long) a;
> -	const unsigned int val2 = ((const struct xlat *) b)->val;
> +	const uint64_t val1 = (const uint64_t) a;

This looks like a bug.

> +	const uint64_t val2 = ((const struct xlat *) b)->val;
>  	return (val1 > val2) ? 1 : (val1 < val2) ? -1 : 0;
>  }
>  
>  const char *
> -xlat_search(const struct xlat *xlat, const size_t nmemb, const unsigned int val)
> +xlat_search(const struct xlat *xlat, const size_t nmemb, const uint64_t val)
>  {
>  	const struct xlat *e =
> -		bsearch((const void*) (const unsigned long) val,
> +		bsearch((const void*) val,
>  			xlat, nmemb, sizeof(*xlat), xlat_bsearch_compare);

This also looks like a bug.  The only reason why it doesn't cause
regressions on 32-bit systems is that the only user of xlat_search
passes a value of type unsigned int.


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