Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1336017

Re: [PATCH 1/7] netfilter: fix IS_ERR_VALUE usage

Show all headers | View raw

On Mon, Feb 15, 2016 at 03:35:19PM +0100, Andrzej Hajda wrote:
> IS_ERR_VALUE should be used only with unsigned long type.
> Otherwise it can work incorrectly. To achieve this function
> xt_percpu_counter_alloc is modified to return unsigned long,
> and its result is assigned to temporary variable to perform
> error checking, before assigning to .pcnt field.

	Wrong fix, IMO.  Just have an anon union of u64 pcnt and
struct xt_counters __percpu *pcpu in there.  And make this

> +static inline unsigned long xt_percpu_counter_alloc(void)
>  {
>  	if (nr_cpu_ids > 1) {
>  		void __percpu *res = __alloc_percpu(sizeof(struct xt_counters),
>  						    sizeof(struct xt_counters));
>  
>  		if (res == NULL)
> -			return (u64) -ENOMEM;
> +			return -ENOMEM;
>  
> -		return (u64) (__force unsigned long) res;
> +		return (__force unsigned long) res;
>  	}
>  
>  	return 0;

take struct xt_counters * and return 0 or -ENOMEM.  Storing the result of
allocation in ->pcpu of passed structure.

I mean, look at the callers -

> -	e->counters.pcnt = xt_percpu_counter_alloc();
> -	if (IS_ERR_VALUE(e->counters.pcnt))
> +	pcnt = xt_percpu_counter_alloc();
> +	if (IS_ERR_VALUE(pcnt))
>  		return -ENOMEM;
> +	e->counters.pcnt = pcnt;

should be
	if (xt_percpu_counter_alloc(&e->counters) < 0)
		return -ENOMEM;

and similar for the rest of callers.  Moreover, if you look at the _users_
of that fields, you'll see that a bunch of those actually want to use
->pcpu instead of doing all those casts.

Really, that's the point - IS_ERR_VALUE is a big red flag saying "we need
to figure out what's going on in that place", which does include reading
through the code.  Mechanical "solutions" like that only hide the problem.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/7] netfilter: fix IS_ERR_VALUE usage Andrzej Hajda <a.hajda@samsung.com> - 2016-02-15 15:40 +0100
  Re: [PATCH 1/7] netfilter: fix IS_ERR_VALUE usage Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-17 03:40 +0100
    Re: [PATCH 1/7] netfilter: fix IS_ERR_VALUE usage Andrzej Hajda <a.hajda@samsung.com> - 2016-02-17 09:50 +0100
    [PATCH v2 1/7] netfilter: fix IS_ERR_VALUE usage Andrzej Hajda <a.hajda@samsung.com> - 2016-02-17 13:50 +0100
      Re: [PATCH v2 1/7] netfilter: fix IS_ERR_VALUE usage Arnd Bergmann <arnd@arndb.de> - 2016-02-17 14:50 +0100
        Re: [PATCH v2 1/7] netfilter: fix IS_ERR_VALUE usage Andrzej Hajda <a.hajda@samsung.com> - 2016-02-17 16:00 +0100
          Re: [PATCH v2 1/7] netfilter: fix IS_ERR_VALUE usage Arnd Bergmann <arnd@arndb.de> - 2016-02-17 16:50 +0100

csiph-web