From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) (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 3FBCD1EE7C5 for ; Tue, 14 Jan 2025 10:18:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736849906; cv=none; b=M8d0f7Fn5+n3Mi7dBkrh7GHI9h502yDTzZT5po/1u/oQtVGLldQqMgSXCDddRl52uIp3VCkD3IKHxbZz3syXbyIMXPERL5CarveQPBAs+Knw/7erWONDU/5jIU8I0SOMBn0kBt/2IaFCVsk8QAFvNNccbw015pekSr3vxBdi1BE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736849906; c=relaxed/simple; bh=Phk8b9T+9WHb4sdBu4TnvawdtCyfivT+01BinaHctZ4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hBGWf5Oa/Sbqc+nrX1CNF/Bvl9+tKz1e9KjK4ZaAspDw4Ar+PK1/tPgxrPoIGLAauShF6Mzo5DHO4Q1IlXn7lZkcyk2vYOc5LMmiSbj9v96XcVs+6Z9gayXcvfRQ2Mqg4prPYbme/uJM+iRwFiYyzluZGAPZ9p0C3qcbmDiKrK0= 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=xurKFT/I; arc=none smtp.client-ip=209.85.218.49 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="xurKFT/I" Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-aa679ad4265so142509966b.0 for ; Tue, 14 Jan 2025 02:18:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1736849901; x=1737454701; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=HiN+MGob9C/wk/ZA5GyHyYf/yvpWUeM+njdEkAZ9s5M=; b=xurKFT/IORv/Svwo/aRFaFpJxYIS0HAy715idWlpOTpFkXLA3LcZnaLZrP2yHkL34d lPGZdyagE/svPtk0ohFx7/Ts0LKDURKWPhgf9D8xqICaQLZspizYOoUSjvVHkP+fMt9m okntnJDC/KxxKUrzypOgedKoU85c+LsV+v/DDBow0M0wjC6HKiHdGdWlCqWXDiZ/G42Z 2MUnFVSWllXAiM9tLtqrRBlOuT+LKDU6XXMs5P7DdE/jkTETsld8fcrtPBSgdNlsQnVP XMbhD7BYzxRizW/LNuhdZk0WPRigmgTrAOxd/AvEx5EX8tc7UQ8ETHPJ4HTx9Eqc0IU/ w4BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736849901; x=1737454701; h=in-reply-to:content-transfer-encoding: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=HiN+MGob9C/wk/ZA5GyHyYf/yvpWUeM+njdEkAZ9s5M=; b=qUXckPGnpiNijP5klXuLyMghm3jTLwC2wxKpfKlFT5LUcCPMDsZRrXyCtnu+MLuzWY xTb5fgk3oipUHadCbKxUXbjbOsWX00CjBXfs2Cbrb9NY+EGhrHgXskYM5DELJ9/jNGck Eqe10Hi/u4WxsSelb5Y4XDhiMctlNujcaeRnsZjneI2Gh1BOyMtWxFJHNbrmiSNofT1M IeCe/6DV32p6RqrC6tU0LaI9IcbCIkxjMJy82Pcj8pT8x8OljeF2FOK16PjrsdhxYPhF gfgaVDgMVzXNxNo5DNfvMZzjUwPB32Ft9dvbQHJ5Enp/JlMJrvqrMPpTCVIy2cGHnnke aP6Q== X-Forwarded-Encrypted: i=1; AJvYcCVNNllodh67DGw3CgFeDoW59hatohByiz144KWvbS59YCEAWft/KwAYYZk4g2GwtnC9BoWj0A3w53f+xME=@vger.kernel.org X-Gm-Message-State: AOJu0YwLXV6Ibims5q/TlYsyMmE2yFDDUbEcuHdEM/y5QYYdAAtAUbHL Zp+vQURzX304se8P9H7OQgnbO+a4yfVcO+F4Bl0L4enmIAMcCcPmkmt1OuDdcvg= X-Gm-Gg: ASbGncvsuuYOvmu9PhOC5GRhcxIBJIq0Wj6MwUxEbUm9o8XrWvdGBef98+lL+hwDCiV nxEdJy8xBkjZJca1tZv4uKt7X3DhAFi8Cj6o9x1CMisRN/W2waP4p5DNUouPRH2zJyZiY7zdso/ Pc7cF0Ktgu/0irpRn6P+PdgHlJoh4uu7Q+5opoJCJG0vo9ljefWYpmFWOkfSnXKxUI2qRmtnu+H RBsipwH2baBPZOYhJB5ekzlVfYgGZ3GCOLlIuR8nm1u/vFGob8QBZXpsZUGlA== X-Google-Smtp-Source: AGHT+IG7WJ9asuOTLZHhddJPlsSkJ0crkaJ1q1rMz0GpfqaRWdDxghBy3ePeVf/MPb7/xuRHzAReRg== X-Received: by 2002:a17:907:2d86:b0:ab3:42b2:2886 with SMTP id a640c23a62f3a-ab342b228f4mr34692266b.3.1736849901536; Tue, 14 Jan 2025 02:18:21 -0800 (PST) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ab2c905cd8asm609773166b.7.2025.01.14.02.18.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jan 2025 02:18:21 -0800 (PST) Date: Tue, 14 Jan 2025 13:18:17 +0300 From: Dan Carpenter To: Namjae Jeon Cc: Steve French , Sergey Senozhatsky , Tom Talpey , linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH RESEND] ksmbd: fix integer overflows on 32 bit systems Message-ID: <2fb3efb4-a889-4b49-8100-51147d9ae426@stanley.mountain> References: 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Tue, Jan 14, 2025 at 04:53:18PM +0900, Namjae Jeon wrote: > On Mon, Jan 13, 2025 at 3:17 PM Dan Carpenter wrote: > > > > On 32bit systems the addition operations in ipc_msg_alloc() can > > potentially overflow leading to memory corruption. Fix this using > > size_add() which will ensure that the invalid allocations do not succeed. > You previously said that memcpy overrun does not occur due to memory > allocation failure with SIZE_MAX. > > Would it be better to handle integer overflows as an error before > memory allocation? I mean we could do something like the below patch but I'd prefer to fix it this way. > And static checkers don't detect memcpy overrun by considering memory > allocation failure? How the struct_size()/array_size() kernel hardenning works is that if you pass in a too large value instead of wrapping to a small value, the math results in SIZE_MAX so the allocation will fail. We already handle allocation failures correctly so it's fine. The problem in this code is that on 32 bit systems if you chose a "sz" value which is (unsigned int)-4 then the kvzalloc() allocation will succeed but the buffer will be 4 bytes smaller than intended and the "msg->sz = sz;" assignment will corrupt memory. Anyway, here is how the patch could look like with bounds checking instead of size_add(). We could fancy it up a bit, but I don't like fancy math. regards, dan carpenter diff --git a/fs/smb/server/transport_ipc.c b/fs/smb/server/transport_ipc.c index befaf42b84cc..e1e3bfff163c 100644 --- a/fs/smb/server/transport_ipc.c +++ b/fs/smb/server/transport_ipc.c @@ -626,6 +626,9 @@ ksmbd_ipc_spnego_authen_request(const char *spnego_blob, int blob_len) struct ksmbd_spnego_authen_request *req; struct ksmbd_spnego_authen_response *resp; + if (blob_len > INT_MAX) + return NULL; + msg = ipc_msg_alloc(sizeof(struct ksmbd_spnego_authen_request) + blob_len + 1); if (!msg) @@ -805,6 +808,9 @@ struct ksmbd_rpc_command *ksmbd_rpc_write(struct ksmbd_session *sess, int handle struct ksmbd_rpc_command *req; struct ksmbd_rpc_command *resp; + if (payload_sz > INT_MAX) + return NULL; + msg = ipc_msg_alloc(sizeof(struct ksmbd_rpc_command) + payload_sz + 1); if (!msg) return NULL; @@ -853,6 +859,9 @@ struct ksmbd_rpc_command *ksmbd_rpc_ioctl(struct ksmbd_session *sess, int handle struct ksmbd_rpc_command *req; struct ksmbd_rpc_command *resp; + if (payload_sz > INT_MAX) + return NULL; + msg = ipc_msg_alloc(sizeof(struct ksmbd_rpc_command) + payload_sz + 1); if (!msg) return NULL; @@ -878,6 +887,9 @@ struct ksmbd_rpc_command *ksmbd_rpc_rap(struct ksmbd_session *sess, void *payloa struct ksmbd_rpc_command *req; struct ksmbd_rpc_command *resp; + if (payload_sz > INT_MAX) + return NULL; + msg = ipc_msg_alloc(sizeof(struct ksmbd_rpc_command) + payload_sz + 1); if (!msg) return NULL;