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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20071220075857.GA1924@ff.dom.local>
Date:	Thu, 20 Dec 2007 08:58:57 +0100
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [IPSEC]: Avoid undefined shift operation when testing
	algorithm ID

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@...dor.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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html