On Wed, Dec 18, 2024 at 05:42:35PM +0800, Herbert Xu wrote:

On Tue, Dec 17, 2024 at 03:32:31PM +0300, Dan Carpenter wrote:

quoted

That seems like basic algebra but we have a long history of getting
integer overflow checks wrong so these days I like to just use
INT_MAX where ever I can.  I wanted to use USHRT_MAX. We aren't allowed
to use more than USHRT_MAX bytes, but maybe we're allowed USHRT_MAX
bits, so I didn't do that.

There is no reason for this to overflow if we rewrite it do do
the division carefully.  Something like this:

I like it!  So obvious in retrospect.  Kees, Justin, this is probably a
good strategy for dealing with round_up() related integer overflows
generally.

overflows to zero:	(len + 7) / 8
      no overflow:	len / 8 + !!(len & 7)

Steffen, this raises a new question: Can normal users create socket
policies of arbtirarily long key lengths? If so we probably should
look into limiting the key length to a sane value.  Of course, given
namespaces we probably should do that in any case.

The length is capped in verify_one_alg() type functions:

	if (nla_len(rt) < (int)xfrm_alg_len(algp)) {

nla_len() is a USHRT_MAX so the rounded value can't be higher than that.

The (int) cast is unnecessary and confusing.  The condition should
probably flipped around so the untrusted part is on the left.

	if (xfrm_alg_len(algp) > nla_len(rt))
		return -EINVAL;

regards,
dan carpenter