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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 93808C677F1 for ; Tue, 21 Feb 2023 21:36:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 054326B0074; Tue, 21 Feb 2023 16:36:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 004446B0075; Tue, 21 Feb 2023 16:36:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE88A6B007B; Tue, 21 Feb 2023 16:36:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id CEF166B0074 for ; Tue, 21 Feb 2023 16:36:44 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A1C571407A0 for ; Tue, 21 Feb 2023 21:36:44 +0000 (UTC) X-FDA: 80492608728.13.D424065 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf04.hostedemail.com (Postfix) with ESMTP id 12FBE40005 for ; Tue, 21 Feb 2023 21:36:41 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=wbr0xfaL; spf=none (imf04.hostedemail.com: domain of mcgrof@infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=mcgrof@infradead.org; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677015402; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BaGXgFyQLBB76avLNaPfA1e27u2MH8f6lBtt4CGYWC8=; b=OfKYunq1SESFkFXIqoIEpWvCU+ZBB4S60Qo12nBgqFGGaDxdabl8YMikDFdKSfpcyj2Wva m9r1XBL0hkIRKgf07C9zzNA5FD0SL9LML5HFaem4Uw1p+xiZ5X3bU3mO8iQgJOB0//mtcO /a0HPyhZ83Tk7yCu0lXn4CBftZbSIsc= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=wbr0xfaL; spf=none (imf04.hostedemail.com: domain of mcgrof@infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=mcgrof@infradead.org; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677015402; a=rsa-sha256; cv=none; b=GZ5LGoz16BC49SdxXNLNvJw41mRR7hv+qmhRSZRkDYh77WDnI+WEo9ZsHIDMhhyMo7JsLM qXQzaXjOGs3FCtw5niEncpu/20AYspqS//JHGR0zHGfebV3ZsuDWaL7xxEOzV9SsZLIO8G OaB46HbtbEHdNyOdfOpS3sHIwI5Bz08= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=BaGXgFyQLBB76avLNaPfA1e27u2MH8f6lBtt4CGYWC8=; b=wbr0xfaL7PNLLFkuCWxuuagZaK /gF5L15kL5mE6TF1g0f2ZDNH2XqbTrbbpet4k7kNdIKKu6VQby3azaLIPrnsMfhVsKfSg2krwr9OG q28HehKcenevUub/DgYTPsOVwq6jaON24x8cW13mYPEgxthaQC4mJfvzntfj31LkVKkk+hj1IPCrE aWrYDTulSlzgvdv614TvvYox7Pw+v9oSpiTp2u3jpLHY9Z/V4qakA10ZDDLmPzdAXpF7XBu5Kgjan HHNbeD0JDbT4RSoGUS718iaFuuJqhSpyb8OUrAcBvXrYT+XMUMlpDKTu5pJPP38i1LyoVl7f92nfY taqwQLow==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pUaJL-009qNI-Tz; Tue, 21 Feb 2023 21:36:31 +0000 Date: Tue, 21 Feb 2023 13:36:31 -0800 From: Luis Chamberlain To: Kees Cook Cc: Ondrej Mosnacek , Iurii Zaikin , Greg Kroah-Hartman , Jiri Slaby , linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] sysctl: fix proc_dobool() usability Message-ID: References: <20230210145823.756906-1-omosnace@redhat.com> <63f500ba.170a0220.c76fc.1642@mx.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <63f500ba.170a0220.c76fc.1642@mx.google.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 12FBE40005 X-Stat-Signature: ruijxqh5awhgy631j94i19jenanyszu4 X-HE-Tag: 1677015401-217445 X-HE-Meta: U2FsdGVkX1/FhqSEty/XY5W9AyLIyTatZXQjXwIq+HP9NyrnQIIPh2t5QejxqsyDgbdEg1DKa4AVehgy4raecfbxGQ0pkpN7tqpGwjbi2M6Wd+GZzcwhCH8hdPyftE4edSnKN40vZ89ICqeJLQQ+ut8/4acD5AgJLXnmEwdyP7xc+xHs0XCTYfgmEr1xeqtLMNqi3c8MqSyTHBReh0kyLYcDnGD8BlAuAltUyYb52eAVGkW6UFzCLLNnaBHQzijvAjyK4fZ1Tn7YLYq3/VWehdIp2dKPYYCkf5XdeG7SlzMr0kH7gf8ODO9xm7F/k//siQ6gFSDu7rZu9WYBm4zzE5sO5pNfvRIqKj7qR+LoGOiVE4Ak3VLornXCuIi7Vl1Br/7q3On0e7bRcKjbtXVH3ekWFAFzW55AIfCTVE/JNBJT7rG4BmfyKxxjJ7e1mIlPEJZ4N+GOdKOUhrHzwjDS/1HAFBAYkhsR8MBMfNWZs/jN6fmbR0U48poe1IOt2HDzc+jYYaLuDHvJ7uJ3qYBgTqYp+qLvCzL3RL6KlXAyddrfhiUCsYcV+XNmIfOsN6/CWGdEkUxLq2vLn98tMofwVQRV693WLVWQUTtoYIlPwtZehbUcqSABz62AAHVx2oP6lWHg2Few1L2DDVtDtvO138d+fv1TKtNaq3LaSwLpyu12GxhBr+bUsE65hL7WhONkUHF1RqL3Gwx7gFjd9luwt5+L7tH8fyxnxEDZd9JAOszH4gDfRY557+3DacB5Puv6ZI6p1xkW8K5lFsMFld55nZgR3F7/3H5qHLXGAiNjEpnRqfE4PV/Q9qiOEa9VjEuDui9YV7tJ+e6ZtS0Z8g2Z2geBEm32PtyLSPmeTZE7+r14ByOM7LjmytjctEKaATHIoRDyXrVb9dQSiI9Aym8A+t7jxaml2unQsHYYOeSh8EHL9YHQiI6I56e8F1oHZMhZSok6cF5E1UaWaYSw4Mq 2Q5bIRT+ CxL/cG38WWh/mY/Tr9CaJVUFhfTSTilCVXKKL7n8ZNrZVXjJmSE8RBHxPYcSsY91HZLoEZvlmOCR3TRiHw9SsGo0EmdfPxN3beMdZppg23XN2jPd6aWiS/aWUII/+OoCByAuwamAJNQYPpSqRyxTkWELlg+Qr/jdLuCjAV9qZrMJ2B106Bx0VVWjkkFzFMWTJnV6YN5ZEgayF9VP175yeUwc9FY/L/vvMv9yN/BrNMdKigumFUVkVKYM9H/uVw82L4Rbf0Ax9JIRyyw+4SepjFN/k/Hmpr07GoXJkrqd1+HeIUhqXD1K1Eoh+uQG+UMjcq2qJIB1LikJujoiuMEZtViTZ16AvTrxtadyOAYMSZ53aW158YUCl2g7wiBEZXeEU129hT9lPVGXuIBJBwbR2VFk55lZDpuajL34/PcW7Oudm2xaTgvW40f7BAJMuOqkJAQbf X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Feb 21, 2023 at 09:34:49AM -0800, Kees Cook wrote: > On Fri, Feb 10, 2023 at 03:58:23PM +0100, Ondrej Mosnacek wrote: > > Currently proc_dobool expects a (bool *) in table->data, but sizeof(int) > > in table->maxsize, because it uses do_proc_dointvec() directly. > > > > This is unsafe for at least two reasons: > > 1. A sysctl table definition may use { .data = &variable, .maxsize = > > sizeof(variable) }, not realizing that this makes the sysctl unusable > > (see the Fixes: tag) and that they need to use the completely > > counterintuitive sizeof(int) instead. > > 2. proc_dobool() will currently try to parse an array of values if given > > .maxsize >= 2*sizeof(int), but will try to write values of type bool > > by offsets of sizeof(int), so it will not work correctly with neither > > an (int *) nor a (bool *). There is no .maxsize validation to prevent > > this. > > > > Fix this by: > > 1. Constraining proc_dobool() to allow only one value and .maxsize == > > sizeof(bool). > > 2. Wrapping the original struct ctl_table in a temporary one with .data > > pointing to a local int variable and .maxsize set to sizeof(int) and > > passing this one to proc_dointvec(), converting the value to/from > > bool as needed (using proc_dou8vec_minmax() as an example). > > 3. Extending sysctl_check_table() to enforce proc_dobool() expectations. > > 4. Fixing the proc_dobool() docstring (it was just copy-pasted from > > proc_douintvec, apparently...). > > 5. Converting all existing proc_dobool() users to set .maxsize to > > sizeof(bool) instead of sizeof(int). > > > > Fixes: 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled") > > Fixes: a2071573d634 ("sysctl: introduce new proc handler proc_dobool") > > Signed-off-by: Ondrej Mosnacek > > Ah nice, thanks for tracking this down. > > Acked-by: Kees Cook Queued onto sysctl-next, will send to Linus as this is a fix too. Luis