From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) (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 F1EDECA7A for ; Sat, 2 Mar 2024 01:51:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709344271; cv=none; b=OE5+a0CSLud55kqdhIkavUNfBydJNuM+j12gUUycB3YMYD4uQf9X8qFF296j4gCsP8kSyzpU4wOV79VBumFYwe23HPYhE0/L6/Lt6DIXfPAK1b0C7QZP+tU4sHDHznQwLHagjQ2DtYCMV4suBc9w9mePIOpRANEVS4RTYXyB1DQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709344271; c=relaxed/simple; bh=/8oSmkPc0IaP5CM6+fKqf9jvC9DwK8RXxd3zPN60QIw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tTvt9NzdZYaqDy1HBpP0ZMUmeU3wyhb2kexzrpizgf1Q6gagNN2cnEpwCR/EbADhQjTP+NHOU+KAFZhZeXN3r7Fvn8JNMB8K4m4OWiMaurnVQGHdQGbKlS5jkDVRit0r0+ummEoCP3dszezeYtxjjKpq8dm5jODJ4r6Ddad/YxI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=TIc1hC1B; arc=none smtp.client-ip=209.85.210.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="TIc1hC1B" Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-6da9c834646so2390173b3a.3 for ; Fri, 01 Mar 2024 17:51:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1709344269; x=1709949069; 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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=TIc1hC1BaG2C6O48gHCVwqAMREqybRwL/38QZkQjs8birZpqFdZpqCIImbYMYW7Eiv jRPIv6GZBhjfsVtfypwxiChRkpef7DVwQMyj5nOOLNLueAgFL8DiRBb4+OjSwCy83w58 nhEfdX/lBy3gXtmcU7KGWUmYplpKX5RVOBkGM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709344269; x=1709949069; 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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=aysB+POhiBdmy2rU2qU2fm3aGahaUWOikNjpoN6+ml32Omyp3Oyc7aDshpMcdBir3t yh1DquAcGtF7nJoL3EiizRHo8AkHN783mDfJgSa9v/GWdy+8RUw2GyVkL6alGW68zqh0 cXIn2zgfJPom5gBmNs1Cqwpd+Agr6mcqG7RotlghB/oDB/J72e1N1ycI7m890z9OLYMr 7QRUe0vReMWyhqJudYeIeR0sZ/XIqcUoGhQA4qQ9e/iHoWjcvEf6uKGzhQpQ1HfrAop1 LORufYWc6h5XASFwIXY4f3L8GpfK5pcgbPC2oYj1LzqzIQrWjQ7D3GU5D5FYZLQjqzc4 R33w== X-Forwarded-Encrypted: i=1; AJvYcCWKPvkmQDwefs0jew9Rxs6AcSI8ZzR6S4Ed2Ik2P85YTpSVy5YTGLOQVxfWxlaji3zlK/87EuDfcfFnk8Sm0tW0MmmXzs4f08yrejY= X-Gm-Message-State: AOJu0YxQyqRlAWk0KO9pVLRjT3/jLyx883hollmlVw++EFjJ9qG+6qDU TfT5bDqjnI+8jWUQMQxYJ0kp3rIS9C6YkWgDwmWOKjjQpTMwqbgun5oSnrXnZg== X-Google-Smtp-Source: AGHT+IHRDk8dRy87orC5sOO0fLYLW475IUp9U1sSARPtPvgR8uAOkuMFjfqikjpTM+PGmCnZ+zOW8Q== X-Received: by 2002:a05:6a20:e11f:b0:1a0:ef1e:a5a7 with SMTP id kr31-20020a056a20e11f00b001a0ef1ea5a7mr3447528pzb.4.1709344269211; Fri, 01 Mar 2024 17:51:09 -0800 (PST) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id rm12-20020a17090b3ecc00b002993f72ed02sm3845854pjb.34.2024.03.01.17.51.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Mar 2024 17:51:08 -0800 (PST) Date: Fri, 1 Mar 2024 17:51:08 -0800 From: Kees Cook To: "Edgecombe, Rick P" Cc: "christophe.leroy@csgroup.eu" , "linux-kernel@vger.kernel.org" , "x86@kernel.org" , "luto@kernel.org" , "dave.hansen@linux.intel.com" , "debug@rivosinc.com" , "akpm@linux-foundation.org" , "linux-sh@vger.kernel.org" , "linux-csky@vger.kernel.org" , "mingo@redhat.com" , "kirill.shutemov@linux.intel.com" , "tglx@linutronix.de" , "linux-parisc@vger.kernel.org" , "loongarch@lists.linux.dev" , "linux-mm@kvack.org" , "linux-snps-arc@lists.infradead.org" , "Liam.Howlett@oracle.com" , "hpa@zytor.com" , "peterz@infradead.org" , "linuxppc-dev@lists.ozlabs.org" , "bp@alien8.de" , "linux-s390@vger.kernel.org" , "linux-alpha@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-mips@vger.kernel.org" , "sparclinux@vger.kernel.org" , "broonie@kernel.org" Subject: Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info Message-ID: <202403011747.9ECFAD060B@keescook> References: <20240226190951.3240433-1-rick.p.edgecombe@intel.com> <20240226190951.3240433-6-rick.p.edgecombe@intel.com> <94a2b919-e03b-4ade-b13e-7774849dc02b@csgroup.eu> <202402271004.7145FDB53F@keescook> <8265f804-4540-4858-adc3-a09c11a677eb@csgroup.eu> <91384b505cb78b9d9b71ad58e037c1ed8dfb10d1.camel@intel.com> <202402280912.33AEE7A9CF@keescook> Precedence: bulk X-Mailing-List: linux-alpha@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Sat, Mar 02, 2024 at 12:47:08AM +0000, Edgecombe, Rick P wrote: > On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > > I totally understand. If the "uninitialized" warnings were actually > > reliable, I would agree. I look at it this way: > > > > - initializations can be missed either in static initializers or via > >   run time initializers. (So the risk of mistake here is matched -- > >   though I'd argue it's easier to *find* static initializers when > > adding > >   new struct members.) > > - uninitialized warnings are inconsistent (this becomes an unknown > > risk) > > - when a run time initializer is missed, the contents are whatever > > was > >   on the stack (high risk) > > - what a static initializer is missed, the content is 0 (low risk) > > > > I think unambiguous state (always 0) is significantly more important > > for > > the safety of the system as a whole. Yes, individual cases maybe bad > > ("what uid should this be? root?!") but from a general memory safety > > perspective the value doesn't become potentially influenced by order > > of > > operations, leftover stack memory, etc. > > > > I'd agree, lifting everything into a static initializer does seem > > cleanest of all the choices. > > Hi Kees, > > Well, I just gave this a try. It is giving me flashbacks of when I last > had to do a tree wide change that I couldn't fully test and the > breakage was caught by Linus. Yeah, testing isn't fun for these kinds of things. This is traditionally why the "obviously correct" changes tend to have an easier time landing (i.e. adding "= {}" to all of them). > Could you let me know if you think this is additionally worthwhile > cleanup outside of the guard gap improvements of this series? Because I > was thinking a more cowardly approach could be a new vm_unmapped_area() > variant that takes the new start gap member as a separate argument > outside of struct vm_unmapped_area_info. It would be kind of strange to > keep them separate, but it would be less likely to bump something. I think you want a new member -- AIUI, that's what that struct is for. Looking at this resulting set of patches, I do kinda think just adding the "= {}" in a single patch is more sensible. Having to split things that are know at the top of the function from the stuff known at the existing initialization time is rather awkward. Personally, I think a single patch that sets "= {}" for all of them and drop the all the "= 0" or "= NULL" assignments would be the cleanest way to go. -Kees -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 90ACDC54E4A for ; Sat, 2 Mar 2024 01:51:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3rNqG4SZ3hQSE+7hymaFVLBX15udqqgWfk1i+v9AHmY=; b=N+oGN0VbOgNpuy cmNzmnSQjRT/NiWSXU8OHlQufn2ftvQ2ZjtCKux2PnWPNDMeJtaTWUThTteh3kGd1ChVpcfmo4qBx k6jJi5IgwIGTycuCPnnwxGJYPeXZpdUco93pLkva4EXrBh1AYoqzc/LXIo6i72RRAfzvR+ZE/AUkJ 9v0x1clcFlk8+DV1GEHUtkyLCvbEjxHgnUygz1TbM+kf+IQkezly7KMkhbAMz1A8eXcy8DYd5TJGt Y9dzsmNRh+ZYXak16cn20rO6fNfOs83mcOPiNx5BhXJ8dwgGm2Wx6RRXcEGaLTEMNusHcyZAFRpQS OykTIpB0TGu3jOa4yZ+A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgEWv-00000002WyJ-3yua; Sat, 02 Mar 2024 01:51:13 +0000 Received: from mail-pf1-x433.google.com ([2607:f8b0:4864:20::433]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgEWt-00000002WwP-0WRS for linux-snps-arc@lists.infradead.org; Sat, 02 Mar 2024 01:51:12 +0000 Received: by mail-pf1-x433.google.com with SMTP id d2e1a72fcca58-6d9f94b9186so2626429b3a.0 for ; Fri, 01 Mar 2024 17:51:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1709344269; x=1709949069; darn=lists.infradead.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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=AM5w1ShjUu0L97c6wQZ075mwgMMDgzOFKatq+FiYlRDb3KGOAq1nP0XHgpyvGXgNtj CE8JqjEEWcSNhBM+PLiWeK4MHCis4r/b9SJHRcV1ebPAzrXm67R/s/v8OWtjsjBqfJ7y +UkVShTzLKBDqUzg25aU1XZN99nmXKyg2cFaA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709344269; x=1709949069; 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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=qzqGVxlLDyGX3y01ZQRfZtZ++8j6LeyMvmMk/Y/+O77V09O2EeNwHK8aNNCxBViHiC 1O6W1zrC4VoTHHBCabVBUKX/zemxsitiJgiXB5KLxTp+TNvW/ZIEyscZMlJecGTd1oFK XHtRpQx92BgygZZlB9u9AHNGmMMwLRDx9xrCy9rU7ZKHp0ZkIpi3PcSRnEU8jQLLODmU kxJjpDdtrb6WTpcrUAirjVKRPtyEDMzoSBoxdWs/VxlYV+2uD81Of67nN0ncmDw9Cxjz yT+rJaQVppsJMPzvCGfkQB2s6zp8T3OdlsBj83tB33KBvG0LcWujWK3VBO8gLM/XFMR1 ESMQ== X-Forwarded-Encrypted: i=1; AJvYcCWi8NBHHCOMsy+OfKN6vHPnRg/58kH8iuGGxcqyODGMZF5ZtLw0/T9b2KZYCX1J389BpIsjExLVvqx+VviwS6popWZdc3ad1zrH+HBhnCCjG8NI X-Gm-Message-State: AOJu0YwPDf9sTi/AzWnzR4YPzypTd2USuByu/4snxJh7m3JhXBD5mlkF z0bcHZu7VLvcAtaXHA2m5rGa8PogFgUBI+rtHBQv3S8wyKuvB+xv1Ht24Sv5MA== X-Google-Smtp-Source: AGHT+IHRDk8dRy87orC5sOO0fLYLW475IUp9U1sSARPtPvgR8uAOkuMFjfqikjpTM+PGmCnZ+zOW8Q== X-Received: by 2002:a05:6a20:e11f:b0:1a0:ef1e:a5a7 with SMTP id kr31-20020a056a20e11f00b001a0ef1ea5a7mr3447528pzb.4.1709344269211; Fri, 01 Mar 2024 17:51:09 -0800 (PST) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id rm12-20020a17090b3ecc00b002993f72ed02sm3845854pjb.34.2024.03.01.17.51.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Mar 2024 17:51:08 -0800 (PST) Date: Fri, 1 Mar 2024 17:51:08 -0800 From: Kees Cook To: "Edgecombe, Rick P" Cc: "christophe.leroy@csgroup.eu" , "linux-kernel@vger.kernel.org" , "x86@kernel.org" , "luto@kernel.org" , "dave.hansen@linux.intel.com" , "debug@rivosinc.com" , "akpm@linux-foundation.org" , "linux-sh@vger.kernel.org" , "linux-csky@vger.kernel.org" , "mingo@redhat.com" , "kirill.shutemov@linux.intel.com" , "tglx@linutronix.de" , "linux-parisc@vger.kernel.org" , "loongarch@lists.linux.dev" , "linux-mm@kvack.org" , "linux-snps-arc@lists.infradead.org" , "Liam.Howlett@oracle.com" , "hpa@zytor.com" , "peterz@infradead.org" , "linuxppc-dev@lists.ozlabs.org" , "bp@alien8.de" , "linux-s390@vger.kernel.org" , "linux-alpha@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-mips@vger.kernel.org" , "sparclinux@vger.kernel.org" , "broonie@kernel.org" Subject: Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info Message-ID: <202403011747.9ECFAD060B@keescook> References: <20240226190951.3240433-1-rick.p.edgecombe@intel.com> <20240226190951.3240433-6-rick.p.edgecombe@intel.com> <94a2b919-e03b-4ade-b13e-7774849dc02b@csgroup.eu> <202402271004.7145FDB53F@keescook> <8265f804-4540-4858-adc3-a09c11a677eb@csgroup.eu> <91384b505cb78b9d9b71ad58e037c1ed8dfb10d1.camel@intel.com> <202402280912.33AEE7A9CF@keescook> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240301_175111_201096_B1B42EAC X-CRM114-Status: GOOD ( 28.44 ) X-BeenThere: linux-snps-arc@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux on Synopsys ARC Processors List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+linux-snps-arc=archiver.kernel.org@lists.infradead.org On Sat, Mar 02, 2024 at 12:47:08AM +0000, Edgecombe, Rick P wrote: > On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > > I totally understand. If the "uninitialized" warnings were actually > > reliable, I would agree. I look at it this way: > > = > > - initializations can be missed either in static initializers or via > > =A0 run time initializers. (So the risk of mistake here is matched -- > > =A0 though I'd argue it's easier to *find* static initializers when > > adding > > =A0 new struct members.) > > - uninitialized warnings are inconsistent (this becomes an unknown > > risk) > > - when a run time initializer is missed, the contents are whatever > > was > > =A0 on the stack (high risk) > > - what a static initializer is missed, the content is 0 (low risk) > > = > > I think unambiguous state (always 0) is significantly more important > > for > > the safety of the system as a whole. Yes, individual cases maybe bad > > ("what uid should this be? root?!") but from a general memory safety > > perspective the value doesn't become potentially influenced by order > > of > > operations, leftover stack memory, etc. > > = > > I'd agree, lifting everything into a static initializer does seem > > cleanest of all the choices. > = > Hi Kees, > = > Well, I just gave this a try. It is giving me flashbacks of when I last > had to do a tree wide change that I couldn't fully test and the > breakage was caught by Linus. Yeah, testing isn't fun for these kinds of things. This is traditionally why the "obviously correct" changes tend to have an easier time landing (i.e. adding "=3D {}" to all of them). > Could you let me know if you think this is additionally worthwhile > cleanup outside of the guard gap improvements of this series? Because I > was thinking a more cowardly approach could be a new vm_unmapped_area() > variant that takes the new start gap member as a separate argument > outside of struct vm_unmapped_area_info. It would be kind of strange to > keep them separate, but it would be less likely to bump something. I think you want a new member -- AIUI, that's what that struct is for. Looking at this resulting set of patches, I do kinda think just adding the "=3D {}" in a single patch is more sensible. Having to split things that are know at the top of the function from the stuff known at the existing initialization time is rather awkward. Personally, I think a single patch that sets "=3D {}" for all of them and drop the all the "=3D 0" or "=3D NULL" assignments would be the cleanest way to go. -Kees -- = Kees Cook _______________________________________________ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 404FDC5475B for ; Sat, 2 Mar 2024 01:51:59 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=QCmB6awY; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Tmny55xWTz3vg9 for ; Sat, 2 Mar 2024 12:51:57 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=QCmB6awY; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=chromium.org (client-ip=2607:f8b0:4864:20::52d; helo=mail-pg1-x52d.google.com; envelope-from=keescook@chromium.org; receiver=lists.ozlabs.org) Received: from mail-pg1-x52d.google.com (mail-pg1-x52d.google.com [IPv6:2607:f8b0:4864:20::52d]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4TmnxH4dt2z3bqB for ; Sat, 2 Mar 2024 12:51:13 +1100 (AEDT) Received: by mail-pg1-x52d.google.com with SMTP id 41be03b00d2f7-517ab9a4a13so2219412a12.1 for ; Fri, 01 Mar 2024 17:51:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1709344269; x=1709949069; darn=lists.ozlabs.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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=QCmB6awYQWrcAyKStNKVG4J4swgSdf5R6JDKxGAplA3r6b6NpTxBWlw9lMPGu/vD9s NWgzxqW2S9qfo3MLs+RHPwVOpoaz+GUgvWp/fg7tBpavPvz/qeSGrR62dkR0Df6zHUwh I7kd8N349t5C7bESq4TSPq85UyJXu6DIZGkaQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709344269; x=1709949069; 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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=ULvhZ65IA6g9ajjfSNnO+IQ0sI87kXoODnqjxn4qNfd5Nq06yMdzRnEVgMK3iHbdaF BwOapbrxU+DuAmL75iX35KrlO0IbyVY4WH3i+cjUrSZkh9EKHgzqB7YmljiD4Fac6TAW A4SavrXb2aZW8iKbmxRx/NDhOmBveCL1IW3gZCEzdpeSIaUN5UQo4QlvZgd4Mcknj7z+ Pc4pGyeGyPGfRSXLJIqex0ah3GnkzIo7VWpo5jhCt5Dl4S8LEEZLS14OqKw59IK6uQJr 3w7iPfDGrPzgRpx6q6XYBdyhzISQDAlhtLPWUAghVw3QQDxTHy7pLlj48Sd5/16TtFSE UA9w== X-Forwarded-Encrypted: i=1; AJvYcCXUZ75imslhcyWi/RbS6JoCKF9qqRslJj32KNWTrsnAiuRP9KQevTtva/ExtKH+aIe3GFO6q+TJKt0uJ6FUDj06IY3LracZOOZDiRVI4w== X-Gm-Message-State: AOJu0YwTeQSjIV0Y7hAhyeFhwJfyCQDaX2CxspiLpx6p2R92pxE2h/ky Y8PqC4J7I0FLRHGLPAj05Bh9rlsX5tCyyaAqNscya7PpmBu7pWVtuBA0G0bpng== X-Google-Smtp-Source: AGHT+IHRDk8dRy87orC5sOO0fLYLW475IUp9U1sSARPtPvgR8uAOkuMFjfqikjpTM+PGmCnZ+zOW8Q== X-Received: by 2002:a05:6a20:e11f:b0:1a0:ef1e:a5a7 with SMTP id kr31-20020a056a20e11f00b001a0ef1ea5a7mr3447528pzb.4.1709344269211; Fri, 01 Mar 2024 17:51:09 -0800 (PST) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id rm12-20020a17090b3ecc00b002993f72ed02sm3845854pjb.34.2024.03.01.17.51.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Mar 2024 17:51:08 -0800 (PST) Date: Fri, 1 Mar 2024 17:51:08 -0800 From: Kees Cook To: "Edgecombe, Rick P" Subject: Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info Message-ID: <202403011747.9ECFAD060B@keescook> References: <20240226190951.3240433-1-rick.p.edgecombe@intel.com> <20240226190951.3240433-6-rick.p.edgecombe@intel.com> <94a2b919-e03b-4ade-b13e-7774849dc02b@csgroup.eu> <202402271004.7145FDB53F@keescook> <8265f804-4540-4858-adc3-a09c11a677eb@csgroup.eu> <91384b505cb78b9d9b71ad58e037c1ed8dfb10d1.camel@intel.com> <202402280912.33AEE7A9CF@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "luto@kernel.org" , "linux-sh@vger.kernel.org" , "peterz@infradead.org" , "dave.hansen@linux.intel.com" , "linux-mips@vger.kernel.org" , "linux-mm@kvack.org" , "linux-csky@vger.kernel.org" , "hpa@zytor.com" , "sparclinux@vger.kernel.org" , "linux-s390@vger.kernel.org" , "x86@kernel.org" , "mingo@redhat.com" , "linux-snps-arc@lists.infradead.org" , "Liam.Howlett@oracle.com" , "broonie@kernel.org" , "bp@alien8.de" , "loongarch@lists.linux.dev" , "tglx@linutronix.de" , "linux-arm-kernel@lists.infradead.org" , "debug@rivosinc.com" , "linux-parisc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-alpha@vger.kernel.org" , "akpm@linux-foundation.org" , "linuxppc-dev@lists.ozlabs.org" , "kirill.shutemov@linux.intel.com" Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Sat, Mar 02, 2024 at 12:47:08AM +0000, Edgecombe, Rick P wrote: > On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > > I totally understand. If the "uninitialized" warnings were actually > > reliable, I would agree. I look at it this way: > > > > - initializations can be missed either in static initializers or via > >   run time initializers. (So the risk of mistake here is matched -- > >   though I'd argue it's easier to *find* static initializers when > > adding > >   new struct members.) > > - uninitialized warnings are inconsistent (this becomes an unknown > > risk) > > - when a run time initializer is missed, the contents are whatever > > was > >   on the stack (high risk) > > - what a static initializer is missed, the content is 0 (low risk) > > > > I think unambiguous state (always 0) is significantly more important > > for > > the safety of the system as a whole. Yes, individual cases maybe bad > > ("what uid should this be? root?!") but from a general memory safety > > perspective the value doesn't become potentially influenced by order > > of > > operations, leftover stack memory, etc. > > > > I'd agree, lifting everything into a static initializer does seem > > cleanest of all the choices. > > Hi Kees, > > Well, I just gave this a try. It is giving me flashbacks of when I last > had to do a tree wide change that I couldn't fully test and the > breakage was caught by Linus. Yeah, testing isn't fun for these kinds of things. This is traditionally why the "obviously correct" changes tend to have an easier time landing (i.e. adding "= {}" to all of them). > Could you let me know if you think this is additionally worthwhile > cleanup outside of the guard gap improvements of this series? Because I > was thinking a more cowardly approach could be a new vm_unmapped_area() > variant that takes the new start gap member as a separate argument > outside of struct vm_unmapped_area_info. It would be kind of strange to > keep them separate, but it would be less likely to bump something. I think you want a new member -- AIUI, that's what that struct is for. Looking at this resulting set of patches, I do kinda think just adding the "= {}" in a single patch is more sensible. Having to split things that are know at the top of the function from the stuff known at the existing initialization time is rather awkward. Personally, I think a single patch that sets "= {}" for all of them and drop the all the "= 0" or "= NULL" assignments would be the cleanest way to go. -Kees -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 86C34C54E49 for ; Sat, 2 Mar 2024 01:51:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ufO7Pr1ybJyA8kOuNZoBrBKoDQPltvjcDJHdCE/ulg4=; b=tGhEu6jh1aBGw8 njEWfw9Xr2ZXmeIi8NFB7gD52YD2FrVxpp7LBRbHBdeOvor/1BWH0dNKtaRvo/hgbrQMf19d0vWq3 m9uORC/9CHVKlVneGeoR4pYvECN6/K4kv12Ss9l5FZ4YTIMdmPCs1pgbP5cIFcwQy4f/vfWfMhwZQ uxdeFCbpGktUJp+O8ihq458ngxJj2iBK2P+Gdt9HJ0NwjFGM+fjX2iJ6TTeUL9slEQ7pRTL13a9EY d/PLOGxXH61g38jnghvF/r4Wk880hGyN0/PrPuXRY4cpv8TQkk4BhvE5XFg9BlPxGkUXmMUF/mW+m a+OmrSqJYam66dYxlqNQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgEWw-00000002WyY-1WHP; Sat, 02 Mar 2024 01:51:14 +0000 Received: from mail-pf1-x436.google.com ([2607:f8b0:4864:20::436]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgEWt-00000002WwN-0Wr7 for linux-arm-kernel@lists.infradead.org; Sat, 02 Mar 2024 01:51:12 +0000 Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-6e4560664b5so2302749b3a.1 for ; Fri, 01 Mar 2024 17:51:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1709344269; x=1709949069; darn=lists.infradead.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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=AM5w1ShjUu0L97c6wQZ075mwgMMDgzOFKatq+FiYlRDb3KGOAq1nP0XHgpyvGXgNtj CE8JqjEEWcSNhBM+PLiWeK4MHCis4r/b9SJHRcV1ebPAzrXm67R/s/v8OWtjsjBqfJ7y +UkVShTzLKBDqUzg25aU1XZN99nmXKyg2cFaA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709344269; x=1709949069; 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=1JOHRODJmattJBY3YtQ+51tfBSOEI55kux6+SckPNNk=; b=WIqT7ktEjKZjVpKYPrFHUy9LsPaJxgjgVMTHDaD/dsmgKyUvJUifoAaRy7mXuKAVru Ox6tc2tve816fN7f3HarJLR9AtzIxB+HwlgiMN06YuqxCtedG43xgsniJxcOyL02aobA uVCZjIq9/StEw5ALyEPxBjhOcZ0mFCsQ9GZk5w05S86T3CerQNnKj8E/Bvl41nqQvKAJ hBACys5N0FD2e8YVQvSOyArdjOzYtsRl6QqoKKtDpQe/BnVEvACNl0c67OHTWoIeHD/G PxMyaQuRG5SdvhA+tfWS+AMTEHWKXOINWciNqyeI7Shp9ne1E7hVMfTla62Ofzpmdjmq LBcw== X-Forwarded-Encrypted: i=1; AJvYcCWYtne7K7hLtaI0xrMUfgAhYXM+XsnGCR26nW2lSgMLOAQBOhk8JSevvfw0RrU4ec2DM6eZ0JiCL+92DyI8pydniIKoCVz6c+NbpvpNYwIVpNBHQFs= X-Gm-Message-State: AOJu0YxHuTh3G/2akXflls7DiSpuUaIx/h2x9RkPxFrnXkpZCaALqRVM yS2hIGnNmZ7NcnkWWlOG8ZGvzDn4V3rnhSCoC9I66yVfDndzX+3FKYPUiWDC+g== X-Google-Smtp-Source: AGHT+IHRDk8dRy87orC5sOO0fLYLW475IUp9U1sSARPtPvgR8uAOkuMFjfqikjpTM+PGmCnZ+zOW8Q== X-Received: by 2002:a05:6a20:e11f:b0:1a0:ef1e:a5a7 with SMTP id kr31-20020a056a20e11f00b001a0ef1ea5a7mr3447528pzb.4.1709344269211; Fri, 01 Mar 2024 17:51:09 -0800 (PST) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id rm12-20020a17090b3ecc00b002993f72ed02sm3845854pjb.34.2024.03.01.17.51.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Mar 2024 17:51:08 -0800 (PST) Date: Fri, 1 Mar 2024 17:51:08 -0800 From: Kees Cook To: "Edgecombe, Rick P" Cc: "christophe.leroy@csgroup.eu" , "linux-kernel@vger.kernel.org" , "x86@kernel.org" , "luto@kernel.org" , "dave.hansen@linux.intel.com" , "debug@rivosinc.com" , "akpm@linux-foundation.org" , "linux-sh@vger.kernel.org" , "linux-csky@vger.kernel.org" , "mingo@redhat.com" , "kirill.shutemov@linux.intel.com" , "tglx@linutronix.de" , "linux-parisc@vger.kernel.org" , "loongarch@lists.linux.dev" , "linux-mm@kvack.org" , "linux-snps-arc@lists.infradead.org" , "Liam.Howlett@oracle.com" , "hpa@zytor.com" , "peterz@infradead.org" , "linuxppc-dev@lists.ozlabs.org" , "bp@alien8.de" , "linux-s390@vger.kernel.org" , "linux-alpha@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-mips@vger.kernel.org" , "sparclinux@vger.kernel.org" , "broonie@kernel.org" Subject: Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info Message-ID: <202403011747.9ECFAD060B@keescook> References: <20240226190951.3240433-1-rick.p.edgecombe@intel.com> <20240226190951.3240433-6-rick.p.edgecombe@intel.com> <94a2b919-e03b-4ade-b13e-7774849dc02b@csgroup.eu> <202402271004.7145FDB53F@keescook> <8265f804-4540-4858-adc3-a09c11a677eb@csgroup.eu> <91384b505cb78b9d9b71ad58e037c1ed8dfb10d1.camel@intel.com> <202402280912.33AEE7A9CF@keescook> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240301_175111_201320_C5D35066 X-CRM114-Status: GOOD ( 30.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Mar 02, 2024 at 12:47:08AM +0000, Edgecombe, Rick P wrote: > On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > > I totally understand. If the "uninitialized" warnings were actually > > reliable, I would agree. I look at it this way: > > = > > - initializations can be missed either in static initializers or via > > =A0 run time initializers. (So the risk of mistake here is matched -- > > =A0 though I'd argue it's easier to *find* static initializers when > > adding > > =A0 new struct members.) > > - uninitialized warnings are inconsistent (this becomes an unknown > > risk) > > - when a run time initializer is missed, the contents are whatever > > was > > =A0 on the stack (high risk) > > - what a static initializer is missed, the content is 0 (low risk) > > = > > I think unambiguous state (always 0) is significantly more important > > for > > the safety of the system as a whole. Yes, individual cases maybe bad > > ("what uid should this be? root?!") but from a general memory safety > > perspective the value doesn't become potentially influenced by order > > of > > operations, leftover stack memory, etc. > > = > > I'd agree, lifting everything into a static initializer does seem > > cleanest of all the choices. > = > Hi Kees, > = > Well, I just gave this a try. It is giving me flashbacks of when I last > had to do a tree wide change that I couldn't fully test and the > breakage was caught by Linus. Yeah, testing isn't fun for these kinds of things. This is traditionally why the "obviously correct" changes tend to have an easier time landing (i.e. adding "=3D {}" to all of them). > Could you let me know if you think this is additionally worthwhile > cleanup outside of the guard gap improvements of this series? Because I > was thinking a more cowardly approach could be a new vm_unmapped_area() > variant that takes the new start gap member as a separate argument > outside of struct vm_unmapped_area_info. It would be kind of strange to > keep them separate, but it would be less likely to bump something. I think you want a new member -- AIUI, that's what that struct is for. Looking at this resulting set of patches, I do kinda think just adding the "=3D {}" in a single patch is more sensible. Having to split things that are know at the top of the function from the stuff known at the existing initialization time is rather awkward. Personally, I think a single patch that sets "=3D {}" for all of them and drop the all the "=3D 0" or "=3D NULL" assignments would be the cleanest way to go. -Kees -- = Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel