From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f53.google.com (mail-ed1-f53.google.com [209.85.208.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 502A71A2392 for ; Wed, 18 Dec 2024 10:54:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734519285; cv=none; b=NjzRLfp4aYEhpSHNtAcVK8kKBspuH2y+ydOLAsk7duQn4MrmhB5epTeisx5bd3ALXjRceYFoEHDalWfOSp9l7a0EsMqXAwWE9Y262Kt5RBbLynch1nLvMdhwYDpvZokLI6pKKFtxhFd1pEV2cP/NY1xQhttENM225YGotCAxEec= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734519285; c=relaxed/simple; bh=31AJvFncyDZ/FXq2NhVl5iGwRm7DlmYnqi4Ssp7vJGs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=e4Kt8D1KJNLwEx1jneAgLxBHz5JXzCz3P1WeTdQseTSIH2xokejVoX9dlHstvd8qXZyApdvmnJMZ0iKx6Kro7UmrZ4/NxDqQVrisHsxGGzsyf7+hA+06SVGZOnmn40pr4nskLkK3QvPWDn1KVQ55bmjZxdjyVNHfqMeejXqYGJc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=rtPXTwQO; arc=none smtp.client-ip=209.85.208.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="rtPXTwQO" Received: by mail-ed1-f53.google.com with SMTP id 4fb4d7f45d1cf-5d414b8af7bso12896609a12.0 for ; Wed, 18 Dec 2024 02:54:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1734519282; x=1735124082; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bhTokHP9skR7jJeKPK9CVtZx11UuslYBY9BBg+qEOjA=; b=rtPXTwQOcdOhb9yqKuMmBbwMgp+UI8vYI/v0419zOm2xRkHD7jt1obZ6FaBLF5cive ZiFJDWgoogsL5UTN5CkxAZzAsjYc6ItrK5lw3MfM5tFRiipDpjxImNHXKEC0OKkstz8c y1C00kv0OZhwGcto8F8nAmRRiF2gbth16ndCJVdCVoEDyBL46x3qG0Q5xbBN2KvRBRBy 9GAR7k/dBaSjOmLEuTTwQHNP70Qz/8JOGga0AVj0Xhw7E4lM9uS+4HqZAxt1kLzkbkuu W+OXX1lwJsdPXCgla9PT0fhZBeA/v2xY7y1qatlOGMBBzCCeE6gcfmeKRejWIbvvZmRI YVaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734519282; x=1735124082; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bhTokHP9skR7jJeKPK9CVtZx11UuslYBY9BBg+qEOjA=; b=aQcNzdXjIURDnJbfV31fe+fb1ua9FZPyMiQ+sOGbwW4DFyCZ54Puw/Q1t9duDViybk llEZDPt6zHRWTEa5OKyvg63D/pGMijOmYa2mZhqv3fy7t4kqC/Rwh+142NIfQaW0Fzvm UTL58PG44MUEjCpvjfI1/g99aVl9MHm1Kh6ESxwcBavFSwehSWTDQSUCryHtTq49vk9I zS1fOLffzISTUyUny5D6OwsrrcKEqCOz/OElLn8ByCyIHJ151+g+Z4R+IFMtcaLIl/t6 UEjDfpSUJmQbPREREEZhLCPaVOJgWpu67sL/y9lU5YrAtSg33ov6kzseOUoOHN2Lju5t fPwg== X-Forwarded-Encrypted: i=1; AJvYcCUvEaXJtEqwfZ67JXidwYSS5mdiQxpKKokbdU+gP2l2fMHo0+/GfWQgAg8bR0u2mDf94hMyZxRnSrcGi7A=@vger.kernel.org X-Gm-Message-State: AOJu0YwmvRVHAv/0oL8zEFq8nYJHX7JRTBV+kO8/NLnvWyynTXJPgfpP 4I4iqRwtItqlOKWmt1GrqOPuEnfr/pSF1i5VRCgfocoWQv8WHuBpm7kC6EHBbmY= X-Gm-Gg: ASbGnctFz+5VAdK0d/gAmYRV3fASjqTEKtlXav5+bexXaA940YKb0nM/J/ruCDsD+/9 wHUNFZELD/dXI0dBLxhuDy69r+5RrSWVFl1+wpiDoiXpD3ba7xax8rZeD2NY9kykTOy/N6CHj2k GxwX6Zih2c9OD87mhGLedsbY9ojB2G0rYXoBYBTKGlIP0mizsvHk7GRGcjPpupHaDLgb6vIQJRk 68eqEkAsRq9xzMy+DYQ9F8+QBmjmw0p066lwrvf2up3V991ekLCVBcWi/+YcA== X-Google-Smtp-Source: AGHT+IGd9c9NZz2gvokmKM5Dng3BnFozJjkFoab/PE7l5nl8JOL6hBsbbn1h4w3R6/1qrT9lM4LqKg== X-Received: by 2002:a17:907:3da4:b0:aa6:8fed:7c25 with SMTP id a640c23a62f3a-aabf474bb0cmr240635866b.16.1734519281678; Wed, 18 Dec 2024 02:54:41 -0800 (PST) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aab9606b3a0sm540413066b.81.2024.12.18.02.54.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Dec 2024 02:54:41 -0800 (PST) Date: Wed, 18 Dec 2024 13:54:38 +0300 From: Dan Carpenter To: Herbert Xu , Justin Stitt , Kees Cook Cc: Steffen Klassert , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH net] xfrm: Rewrite key length conversion to avoid overflows Message-ID: References: <92dc4619-7598-439e-8544-4b3b2cf5e597@stanley.mountain> <053456e5-56e7-478b-b73e-96b7c2098d07@stanley.mountain> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: > > > > 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