From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AIpwx48aHd3UeV9UR1ar2g8pUAbhMRPdXZvlpJSwL53glqiFmwE8xtqtMqoYSV3489vgNRsajEh9 ARC-Seal: i=1; a=rsa-sha256; t=1522194355; cv=none; d=google.com; s=arc-20160816; b=sC3Fu362cJpGFsew37IvnkrjJ0dJ4NfC9RV8CS7bV8AFQz4YZvRu6C6biihYIrz7mP 31vEItUPb7kmp7qWpqSNcseJkA3Wtv3v2gAwzq7VbYYeuOrgS5ycAQU4hJimMfvumIma o9c2ZptWzEuzbe3+zbx7vGyWKdpSRh5AhLY39n8PydG5NeeDdCZWPD9g6iXB1fxfuwEr ImGTu7X6yb6TJ8DLJTXr8yoRagMlmNkhRKAxdR/aZWnTErysjeuhjn2eDC9c8POQgodJ JOvMQtqB9rg92NdVM3RItVsIl2EQCLlTKy93RZNnUzSfezGhM4AMt4Jd62H5F8/W/E7e NW+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:delivered-to:list-id :list-subscribe:list-unsubscribe:list-help:list-post:precedence :mailing-list:arc-authentication-results; bh=IGKKHR8Ozfyj1gRHR0mkjBxPjB+xC58LElFTBiBr4TE=; b=pIiYGG6gyue0+85p4lmqtt+rOXHgtSvg3bk26rRAaf5GXpIdHszQSiKuA3uLEuZM5h +zBui5XJOaBCg5+CLdNgbJJsD+IR0u67ZyKxe07szgds1RXepptDpHigBJKcGZHGewHq IMcZVqdT5MJEBJzwgfmIa9FkOi2BlMuhPbi+rXLV+tmR6S+NBqQwxhLAmxWTPcZlT7K6 2YQitdUyJEKQxZgX3DS/hzNSjSGOZHUH/ayn9a2EY5/+6Xnerjn7Yn9BZN47rrMXgG2Z 0zz7w8Hqn8VOSh6X74vbT3Jvc3S0uvmljtcguxBLosaUAG0ZmRJQJwhwgawOv8nHI9Fy mTKw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of kernel-hardening-return-12781-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12781-gregkh=linuxfoundation.org@lists.openwall.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of kernel-hardening-return-12781-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12781-gregkh=linuxfoundation.org@lists.openwall.com Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm List-Post: List-Help: List-Unsubscribe: List-Subscribe: Date: Tue, 27 Mar 2018 16:45:32 -0700 From: Andrew Morton To: Kees Cook Cc: LKML , Segher Boessenkool , Thomas Gleixner , Kernel Hardening Subject: Re: [PATCH v2] rslib: Remove VLAs by setting upper bound on nroots Message-Id: <20180327164532.796cdd043fece53cd9b56f8f@linux-foundation.org> In-Reply-To: References: <20180315225919.GA43806@beast> <20180316155950.35fda7c63ec59277ba504201@linux-foundation.org> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1595046396754035881?= X-GMAIL-MSGID: =?utf-8?q?1596136468508941156?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Mon, 26 Mar 2018 16:17:57 -0700 Kees Cook wrote: > On Fri, Mar 16, 2018 at 11:25 PM, Kees Cook wrote: > > On Fri, Mar 16, 2018 at 3:59 PM, Andrew Morton > > wrote: > >> On Thu, 15 Mar 2018 15:59:19 -0700 Kees Cook wrote: > >> > >>> Avoid stack VLAs[1] by always allocating the upper bound of stack space > >>> needed. The existing users of rslib appear to max out at 24 roots[2], > >>> so use that as the upper bound until we have a reason to change it. > >>> > >>> Alternative considered: make init_rs() a true caller-instance and > >>> pre-allocate the workspaces. This would possibly need locking and > >>> a refactoring of the returned structure. > >>> > >>> Using kmalloc in this path doesn't look great, especially since at > >>> least one caller (pstore) is sensitive to allocations during rslib > >>> usage (it expects to run it during an Oops, for example). > >> > >> Oh. > >> > >> Could we allocate the storage during init_rs(), attach it to `struct > >> rs_control'? > > > > No, because they're modified during decode, and struct rs_control is > > shared between users. :( > > > > Doing those changes is possible, but it requires a rather extensive > > analysis of callers, etc. > > > > Hence, the 24 ultimately. > > Can this land in -mm, or does this need further discussion? Grumble. That share-the-rs_control-if-there's-already-a-matching-one thing looks like premature optimization to me :( I guess if we put this storage into the rs_control (rather than on the stack) then we'd have to worry about concurrent uses of it. It looks like all the other fields are immutable once it's set up so there might be such users. In fact, I suspect there are...