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 625F2C64EC4 for ; Wed, 22 Feb 2023 07:53:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BAA156B0071; Wed, 22 Feb 2023 02:53:20 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B5A9F6B0073; Wed, 22 Feb 2023 02:53:20 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A22E96B0074; Wed, 22 Feb 2023 02:53:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 940AD6B0071 for ; Wed, 22 Feb 2023 02:53:20 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 5446DA0BA4 for ; Wed, 22 Feb 2023 07:53:20 +0000 (UTC) X-FDA: 80494162560.11.4C22939 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf26.hostedemail.com (Postfix) with ESMTP id 251ED140012 for ; Wed, 22 Feb 2023 07:53:17 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LQpfLLLg; spf=pass (imf26.hostedemail.com: domain of omosnace@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677052398; 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=ljX43G0wEY8L63/ppMMsMkg7cRFEwM1I5WynBYgGF5o=; b=Xx/y4Vdo7NPDj4JVucg1LJucaAa+ZYda9jvLTWFVGucH936GjHjkecMN5cHNaqNkiQnLZO diz03+7DGfrCi635R7dHm/4Hik5Jnngx2kgDNzHMx2O94ikzb/7wv7kLLVhaJ39/H31ZVv 9mrFrw/Fa5Awiv7jWQ6Uop7rQ9XFUFE= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LQpfLLLg; spf=pass (imf26.hostedemail.com: domain of omosnace@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677052398; a=rsa-sha256; cv=none; b=0bQghQlJEqTu/p33PUvzjd8SNbwzkKeaAbw7b67W1MEe2LpmWe0ML53/zTkOUo4RdF4hpL ZAJXocZKqPttP81/OYek478JEvKOH/y5Juc43SE5GIbs/9RtUd8+Q0ZMmNckrvVSrkcgPn w4cg+pdXBb5woKu8OlWEv9cXsm22E9w= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677052397; 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=ljX43G0wEY8L63/ppMMsMkg7cRFEwM1I5WynBYgGF5o=; b=LQpfLLLg8qAupPwETi+u7tr1nhXMR5BXQmADZ9dyvxg/tk6mHZQ3jvEhBP3WfDsJWZaSKx Dg5G0+Ei2+movroSL2fAU6u29lqR1R2hzqYLlAJu4TiKGOdr0Z15V6xPddN0236maND/68 M9PL84GNqnjCeWlwYVI62PoOzYY9/p8= Received: from mail-pf1-f198.google.com (mail-pf1-f198.google.com [209.85.210.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-536-0KahWfbyOfamAnWdrs-BrQ-1; Wed, 22 Feb 2023 02:53:06 -0500 X-MC-Unique: 0KahWfbyOfamAnWdrs-BrQ-1 Received: by mail-pf1-f198.google.com with SMTP id w130-20020a628288000000b005d1f4325e2aso1494272pfd.18 for ; Tue, 21 Feb 2023 23:53:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677052385; 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=ljX43G0wEY8L63/ppMMsMkg7cRFEwM1I5WynBYgGF5o=; b=PQpR3sNG4yPwEaseBh6rA7Ahq3shutr3z1K0DhOarUUspg4J8NjFFrDE7aNdaAyAQr i3vX8iGgntfOalJ8WrE/6+CDdmRoiqsGF3TdTOTqbKgCJFuZa98kvufUvpwTrhxTJ1Us xJsf6JN3gC0yZ0LsCWuslytGSf5kaVoP7SbQUa8czOh5qqz5+kjK48OaZu+c1Y3vSsEx IThi/wGTIoVjf7YnY4pPGhgHeSFHXu4p2L17PdNlN0fNsVoEuMC7j6MsIMKIxGU5BuLu Ae6HX0ZRxI7mbC9nPm+mhz80R8KtKrzo2aCSpsH4lkf6UIig2KyWQGKsB22hvOPnvB1W /IPg== X-Gm-Message-State: AO0yUKXbktjznl0bQEvrShb5Ic9VvXTtr+Gugd6azCW00h/yui86dwAj hOmmRcKcfHghJ7QGWgwqCwT11uOf3+TFRRsd8yBT6n2qouN7BrfNgGbhzYm6G14L7ep94kysWZa km7JBCNuIgh3wHBuXyLmA8LLhyh0= X-Received: by 2002:a62:e906:0:b0:5a8:c0e0:3b2 with SMTP id j6-20020a62e906000000b005a8c0e003b2mr1331081pfh.45.1677052385599; Tue, 21 Feb 2023 23:53:05 -0800 (PST) X-Google-Smtp-Source: AK7set/h+aPFUm4SbrYbWk3iTTUpShGxZ6VUSjD4ik+PIQ2xgY3zYr2/zd6r5vbnkirTo15iRgIAj/K7nLMGyrknNUw= X-Received: by 2002:a62:e906:0:b0:5a8:c0e0:3b2 with SMTP id j6-20020a62e906000000b005a8c0e003b2mr1331073pfh.45.1677052385289; Tue, 21 Feb 2023 23:53:05 -0800 (PST) MIME-Version: 1.0 References: <20230210145823.756906-1-omosnace@redhat.com> <63f500ba.170a0220.c76fc.1642@mx.google.com> In-Reply-To: From: Ondrej Mosnacek Date: Wed, 22 Feb 2023 08:52:53 +0100 Message-ID: Subject: Re: [PATCH] sysctl: fix proc_dobool() usability To: Luis Chamberlain Cc: Kees Cook , 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 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 251ED140012 X-Stat-Signature: it4mzjpiipaiywf78zhw4eduxg1ua3ho X-Rspam-User: X-HE-Tag: 1677052397-297428 X-HE-Meta: U2FsdGVkX1/qo09HDLk2qezn23byqAmPu6khvGu0M8Rfel+2GbtYbuXJP8Sp9hn0j34u6/7RPr1/xzgHyZI6An4kpqVzDyMW0qi/T3aAmj2oXx5tbk9iUCj/jJ4cLxSMM941qivjbPS6n1hojLfyq9rKjuMPG1lyTC5TLyMRb+xMq6w+COhS7RYKgSrdUDEhAqTW9DPOgVO9xr6PgCxSHp3O1978lmJFbELelQUU5KrLQOtl5SwvrjEg/scBg3PHhH5nLiCHsr4ReGjUQ2VJ9fSFVJIwAraY9BZpQTaGVT/jdSENGYiES/Xs01NxnBj6+c7yZiDVkanjcoQAOSHKi/OXNvzT+SO/0MefcbT1mHzcMOux6L567EtaKmZY1zahznVJz2MesLqkyajpw02SHh8Q0cNdocGypz/Y1eZ/dsV7hDwmlu1EXQdAhM0t4WSVLG44cDsbmmsZ2hWY7KPlRJ+RDsJJyu5Mb0Gf5H13WJJrWSL9YSr9wyYRo7BCkSxFMj99x5B8Qz0mbxubR6tGKD0eezsR3//Id3Jwu9fYXqQPT2l1swfEyxuzQmdPAv9nA3GuzBLC+aB0zLbnYjbA0q+MYLetnOnC293j47vpYKQi9YYS0e8HahBgVj3i7/lS2IAfU6/SR2lA1s5srg5f3KCEK4oB68Wvupb8Qo6YM0u+gvGyBWtDnrChDpNKTFjQPo3qO0Ja55SmdR8rxy9BpfQaYP439tVo6ZCF6Zma8DevaSUrKDSt+7jquunmcpR/4iLZ/bfzd5sahVBASJzp6jeh7QZLSkRAJb5c/eTFEFmKQaSuLBf5uA+1iIB32Bfnsr2Q6cg8X0z4jQB2w/EKy2lj1501owFD+ZgQ8Zh6syLAqrbAt+T+TCZt/bNQTMDQsPB/L7j1Q4W8+2rxXvPVnStTljWmrjE8igvcARq1ThxfAYzdSVYDRnOUT4ySpxX63QptJ4/ocW5C9VIW3/+ IIvQjKgm C0AXeqobnp/bgWqYdXof+cFYljNuVkaAuYfUZRFCC5EGgmjUxPGU5QFDEYtKjAo5msjYlNRt5StgkS7WCqCDzWwrWCnVD3yRdVjdc5nl/ZE/ZIt7Sqb/D6CzkT6nHoRZPpzozvobzIpP9qonPvCcCmkby137e71i1aQdys1TFPPWEzLYOOesiI3xhbEnrV+2GEw8+t8abF8Pd+BvsKtV3y2pTYFzRoaofwbtJ/8V3pdBtq1fBkFEme6WMOsIejlNlnw2mFpeE4en44a8zQcCoW8R4YOWsgO203CcJlVZ4saSQIOOobe50M+3YxJXl1vNg6ZcAznIUaRoVNDeIjbzgyxB1UclPa72GE7eI7IPKDUEFciJaLoAXr5AhcKzebqNS0BpjTECrDhBZWo4rwgk/bOYwiaazaXQRrme8IeY+/POJ0fyuStpGhROukyaRGUVUGamf 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 11:04 PM Luis Chamberlain wrote: > 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. Thanks, Luis! -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.