Re: [IPSEC]: Avoid undefined shift operation when testing algorithm ID

[Jarek Poplawski <jarkao2@gmail.com>]

On 20-12-2007 05:29, Herbert Xu wrote:
> Hi Dave:
> 
> I had wanted to fix this for ages but kept putting it off and then
> forgetting about it :) So before I forget again,
> 
> [IPSEC]: Avoid undefined shift operation when testing algorithm ID
> 
> The aalgos/ealgos fields are only 32 bits wide.  However, af_key tries
> to test them with the expression 1 << id where id can be as large as
> 253.  This produces different behaviour on different architectures.
> 
> The following patch explicitly checks whether ID is greater than 31
> and fails the check if that's the case.
> 
> We cannot easily extend the mask to be longer than 32 bits due to
> exposure to user-space.  Besides, this whole interface is obsolete
> anyway in favour of the xfrm_user interface which doesn't use this
> bit mask in templates (well not within the kernel anyway).
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 878039b..26d5e63 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2784,12 +2784,22 @@ static struct sadb_msg *pfkey_get_base_msg(struct sk_buff *skb, int *errp)
>  
>  static inline int aalg_tmpl_set(struct xfrm_tmpl *t, struct xfrm_algo_desc *d)
>  {
> -	return t->aalgos & (1 << d->desc.sadb_alg_id);
> +	unsigned int id = d->desc.sadb_alg_id;
> +
> +	if (id >= sizeof(t->aalgos) * 8)
> +		return 0;
> +
> +	return (t->aalgos >> id) & 1;
>  }

Hi,

you probably have forgotten to mention in the changelog the returned
value is changed to 0/1 btw?

But, since you've mentioned different architectures, maybe it's the
good moment to find out why you and/or Linux doesn't seem to use
something like CHAR_BIT instead of this 8?

Thanks,
Jarek P.