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 81475C636CC for ; Mon, 20 Feb 2023 12:52:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AFC986B0071; Mon, 20 Feb 2023 07:52:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AAB806B0072; Mon, 20 Feb 2023 07:52:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 925B96B0073; Mon, 20 Feb 2023 07:52:41 -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 7B1F46B0071 for ; Mon, 20 Feb 2023 07:52:41 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 508D71A084E for ; Mon, 20 Feb 2023 12:52:41 +0000 (UTC) X-FDA: 80487659322.28.8741B36 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf12.hostedemail.com (Postfix) with ESMTP id 5D35540002 for ; Mon, 20 Feb 2023 12:52:39 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="Pi27/Cws"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf12.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1676897559; h=from:from: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=mi2k6ov9PIEEOKF8HtnT+0K1MGZyhTRbqIFEKucrAxQ=; b=aMzDEZIY0pvNf47e2WvIlb6zufXDEX4J8bmDYNDIMK1pYd1fiUXewi7y9QH9s3qG6XHxZc CY5MEkgoHLcd3xm3QKhedeFU0IYSxYhhlgkyB1C5C3pg9Z8nPLPKpaWFw0qSfArjF60+2r aTFSJquPJq3dbmavuG2OuTRtfdlvVtk= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="Pi27/Cws"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf12.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676897559; a=rsa-sha256; cv=none; b=Bc4/C6m/t57jb3PXnw92WC8AtPyhUhTPfcyX65EiKKL9Ge4sHVKdAxHv+z9dfudaFodzNq jKmuiG/TDV4WsPSOZOY1wqvaMeQVi+f2fgEiT3QEcM9hVbDEFAccjYAmqp9NxwVIVFZXLe v+a9V3wDRzcUH5eKXn5MNlGY/cAyKzw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676897558; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mi2k6ov9PIEEOKF8HtnT+0K1MGZyhTRbqIFEKucrAxQ=; b=Pi27/CwsY/H/a2ClsPQY4Dak9WhqcJ00dblzmrrxhUeiPYufyi0mTLx/0yOw7D8qoi48e4 xswS1ULkSOpVhUMww3SOD3E1u7j/EMCIbUQSgAmjaG2sGI2B9eBcF5NeXo2BpiOPMM6zxE Mf6Lk9Y+wCkZMmtqTI5z5F74Uuyqpfo= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-153-vcBOQtfXMfmnCacMs36yAg-1; Mon, 20 Feb 2023 07:52:37 -0500 X-MC-Unique: vcBOQtfXMfmnCacMs36yAg-1 Received: by mail-pj1-f72.google.com with SMTP id u64-20020a17090a51c600b00233cde95f87so647223pjh.9 for ; Mon, 20 Feb 2023 04:52:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=mi2k6ov9PIEEOKF8HtnT+0K1MGZyhTRbqIFEKucrAxQ=; b=6wvPl/S8zT6EtApq99goA/QoY1heBlgzRNSqRe5dyrAvc9H9xffdLbGWwRvnSEJFf0 BQ9uKsS037DmxjgOqCIfKUn1axUY9sUU557Z9MUY7PJgzXsOzAvEd9eQNoqgePmDiszJ cU8pwKw4U99EnfC++bx+uhxxARCYxPznfPRliVbh+tr1DbxJ7+cm3YATgOSywA9Bwe4T 8AkTn+XHs5YIl9tRPdL3XgMb6YaU3MNdC4jeEJcsxdXNe5B8VI3XtMJkJtbdco/H1+/+ X6qjOEuaGbGfOfznp5UyTMg8ZGkbujR5nWqv6j5O15CWBKI1RbVVwsHMZeNO4nacsVNO Ytpg== X-Gm-Message-State: AO0yUKUr1+DZYsVQTjxaffNi7z6C9d+UX4SLNdQbBMdq72JH6DGDHjXx ISkyLEHY7quqJwaqxNLl1kVOwpZmOT22sAPjtTUcuVfVCEmf2sGjr3nPhKDxieO758L9QXH6gZQ oYZn+elIIBoMQ1w4kLX5gKgc0kyg= X-Received: by 2002:a17:903:449:b0:19a:f63a:47db with SMTP id iw9-20020a170903044900b0019af63a47dbmr191506plb.2.1676897556571; Mon, 20 Feb 2023 04:52:36 -0800 (PST) X-Google-Smtp-Source: AK7set/HmWnP91UrLVg24xciP71WBRN1oXKx7zdnbGZIUV6VrXz5Bnk6LyVvuteQq63xg3d0WGyePBM+ET5m+YQOvdI= X-Received: by 2002:a17:903:449:b0:19a:f63a:47db with SMTP id iw9-20020a170903044900b0019af63a47dbmr191497plb.2.1676897556296; Mon, 20 Feb 2023 04:52:36 -0800 (PST) MIME-Version: 1.0 References: <20230210145823.756906-1-omosnace@redhat.com> In-Reply-To: <20230210145823.756906-1-omosnace@redhat.com> From: Ondrej Mosnacek Date: Mon, 20 Feb 2023 13:52:24 +0100 Message-ID: Subject: Re: [PATCH] sysctl: fix proc_dobool() usability To: Luis Chamberlain , Kees Cook , Iurii Zaikin Cc: Greg Kroah-Hartman , Jiri Slaby , linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 5D35540002 X-Stat-Signature: dusyyzikeh5d47jkfcmupnfs18bpyn7d X-HE-Tag: 1676897559-948006 X-HE-Meta: U2FsdGVkX190OspzQbRXuoCEi7tRqC9Yl8c7mL9ibT24SYta+7MS4HrVa9UkJXFc1CWPn5eSLZIxxBenR8HjOohiRmR4/jef1mFsJfz+PjnkduEhReLpwj0Pviu3FxjaStoYuHjk6nrQ3cFiePJ1jaetFosdeH2HhPsYRMSdzRwfJUw+bHkcgyXdDHyk6fGXleNuQbVKR74L4FJQXAILs4pIWbbYsoNMdKbYo5GuUxyH+3xrDOV9RiJnqOy8ivBiUX26oXFEgQLKN5xZcsYWxstF21gc8V6IlMRO4nsgALnpI7iwdSs+bxEwqNdN3OBAxBKIKFLw7tdQ3BGNwlgwovSji+xbYDjPqYV/3GvD7lvkAgIkMAbvVJrGG0bx0hRJ6c9IrG92BRVwOUdQeySDrMswkVk3yj+c5yjPtlvZiNMzdjQhWgIo1Vtr1m17I9LZzGcsWBlhfCr6mAiWrlH4FzF4PGUpDE2OIY5/xFTitR+DNGD3HnM7EksePbx4UOnCAp4BcXU1KyoDy7dOpSwqC4UAGriN6XOWVYIdoFHDLRJvzHkZGCryoNr+xk9mzHGVZsXPzt8ueqhvK7wA5h5T48/UHCKaKFe6RMHe/6s1m6FEgtsLbNMfe0xof7I/r/JeHeHSjEFPDoc+nqLrRx2K2Onus8KpKp8/qWXC6qRq4nQdGiegHqY/W+djJlragQE1ANJdpXrLZIwPHTYFHGnT5aamZpiRJBAa5KTviCGuCT2OH3rlW1W+fPklJvUNypskIHFCj2asiMl585sSa7w90J74nGjMGWAv6XxO3ckTz6wFK7Wq16pCsz3Fdi6DHrGgTYRx5rUojWHRbcWMMyb1fqUR+o4ybIeDPtZl3PUgRJS8h3SZMCaeHaFx96uY46uZUIByrh1ZwmzR8MNiwdAOF6+CQOlHHqB3e64C+HH6vzAH0TDUCaiJ4/SHunDr8MZTHRMguC6b7PAd97BQKPD pRq8vJ1Y ub/+26T88htvqCtGzlQly+SQ6EF0DMhXPpaDWbZtETFqJ28ebprINhbIcUlcr8hNnmrRW1F8K+JemBLzpGzmd647yZkmad9+Bo09t+ydCfzWuVF6PNFZfyhG90xWqLtptJ+4mlBI9cdOyb7mwFe9CfXByssgTTWhpZr9eSl/+VZdq1/nLpnOC1KmwaBHOP9O5pKjSw5Zc+HdD4UHFWRP06/i0TqQpSK/a52ycgu/qGugMwyCIYjBtfEJzMxpvAn2hpDkNzmfAfuKGwzDXMMf6Kt3miP36xQfw6I2nAeLlI+oBOYesE1QBuM+Y2NorJmMICLetdx52RtVmq5KCPRFAKnAMEVGzXSRZ7c3le1pz5ErIcSSylJL6GGf+a1lwgfNk38VoyUfAcFdF5QYKu+hriKJ/1bYls/IlSJUTBNL7VS4tL3Q5krVIN8NjOmyoEC3vbEhiPZzqEZvy+TM= 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 Fri, Feb 10, 2023 at 3:58 PM 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 > --- > fs/lockd/svc.c | 2 +- > fs/proc/proc_sysctl.c | 6 ++++++ > kernel/sysctl.c | 43 ++++++++++++++++++++++++------------------- > mm/hugetlb_vmemmap.c | 2 +- > 4 files changed, 32 insertions(+), 21 deletions(-) Gentle ping... Without this patch the new "dev.tty.legacy_tiocsti" sysctl is unusable. This is blocking me from making selinux-testsuite work with CONFIG_LEGACY_TIOCSTI=n: https://lore.kernel.org/selinux/CAHC9VhQwrjwdW27+ktcT_9q-N7AmuUK8GYgoYbPXGVAcjwA4nQ@mail.gmail.com/T/ -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.