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 24DABC433EF for ; Tue, 15 Mar 2022 18:39:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 999048D0002; Tue, 15 Mar 2022 14:39:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 947B68D0001; Tue, 15 Mar 2022 14:39:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7E7FC8D0002; Tue, 15 Mar 2022 14:39:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.28]) by kanga.kvack.org (Postfix) with ESMTP id 6BA898D0001 for ; Tue, 15 Mar 2022 14:39:26 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id 4578261988 for ; Tue, 15 Mar 2022 18:39:26 +0000 (UTC) X-FDA: 79247483532.08.B547EB2 Received: from mail-qv1-f48.google.com (mail-qv1-f48.google.com [209.85.219.48]) by imf15.hostedemail.com (Postfix) with ESMTP id 807A1A0008 for ; Tue, 15 Mar 2022 18:39:25 +0000 (UTC) Received: by mail-qv1-f48.google.com with SMTP id gm1so185375qvb.7 for ; Tue, 15 Mar 2022 11:39:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=c6BjYFfCINr1jbMqdvsvHsZfVzgMAvQu+FmAJGwHfQk=; b=LCi1mMZcSoeMOKheGc5YJbXIWo9H+JBKs1bU/F9v3Am+bgzh+6HZ8Kypkam3Fx9Ik3 YL31XZCUPjwNJqiGpVkrkrK0utu+y2CDUPG9WQuWtIws9kguAu0/Rvwtj42BXTs2kH0v gr62iJW8cpA977drZHURZOBcvDhjJdvmCg0wJVBgToeZsMqoOPzMMXkLJsMS5kKuKy8/ xtdNKlTAIO6MSD6ke3pJ/2gGtLUcp2WDqpN8h3giDGH0oK8WVzSS4dL7NLMcCARBNtYM 9h+CdpzZRpRkS7fKu3csx88XO2KdW2e3iQvEYvklT/d92JE0nmx41ssGIoWKzgZjAlGE 4R5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=c6BjYFfCINr1jbMqdvsvHsZfVzgMAvQu+FmAJGwHfQk=; b=Hi6HLSgTuteVIVui+Mb9avUWhCxTUB5q98Uj+u++o9Q9MbtzDg80Db2g/l5jrHfCWB uyK1By3NMa4sq5SnaDMvbU7Akrx3/bJ8Js/jCzWBUP3vSd1QibMduSl0xvEcPpmfNVQI uucOhgEI3J0dyd25mIRDdQ3wQa1t2COINmXlLySIUvXc6e5K2+UnMklb+hAcBP3tCEzF dANpFbSvOrDKJTuE9HzHRGEd2j9Ap1REnh7weswJ1agiV5PYAZyVNDHZJmGlkfaPrISb RguBoI6OdnJJveK9Mon2d/zDzCe9ipCoVa0MN+8dJXLBDq6aTa6MM8vCV4ZcskY3Li4E B7Jw== X-Gm-Message-State: AOAM531dpebJ85d8B9LlFyaFy2eP4NVmvTZW/wrxgaWi+TaegG5utt8A bX5+1vrS9EtiVBjKENvvUmXjZg== X-Google-Smtp-Source: ABdhPJzWBj4X5F6V31ECL1lv0nh007pV8FVtZk5v4tU1J1WdGNdd7Zb08gS086sB0gd9msCqcunMxA== X-Received: by 2002:a05:6214:3016:b0:439:365c:56b6 with SMTP id ke22-20020a056214301600b00439365c56b6mr19278622qvb.47.1647369564762; Tue, 15 Mar 2022 11:39:24 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-142-162-113-129.dhcp-dynamic.fibreop.ns.bellaliant.net. [142.162.113.129]) by smtp.gmail.com with ESMTPSA id e7-20020a37ac07000000b0067d7cd47af4sm6473808qkm.31.2022.03.15.11.39.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Mar 2022 11:39:23 -0700 (PDT) Received: from jgg by mlx with local (Exim 4.94) (envelope-from ) id 1nUC4o-0010R3-Jf; Tue, 15 Mar 2022 15:39:22 -0300 Date: Tue, 15 Mar 2022 15:39:22 -0300 From: Jason Gunthorpe To: Mika =?utf-8?B?UGVudHRpbMOk?= Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, apopple@nvidia.com, jhubbard@nvidia.com, rcampbell@nvidia.com, vbabka@suse.cz Subject: Re: [PATCH v2] mm/hmm/test: simplify hmm test code: use miscdevice instead of char dev Message-ID: <20220315183922.GC64706@ziepe.ca> References: <20220311033050.22724-1-mpenttil@redhat.com> <20220314182439.GB64706@ziepe.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 807A1A0008 X-Rspam-User: Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=ziepe.ca header.s=google header.b=LCi1mMZc; spf=pass (imf15.hostedemail.com: domain of jgg@ziepe.ca designates 209.85.219.48 as permitted sender) smtp.mailfrom=jgg@ziepe.ca; dmarc=none X-Stat-Signature: bm1rpcuia45m6gf6gatu8f4j3qsek6fq X-Rspamd-Server: rspam07 X-HE-Tag: 1647369565-878660 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, Mar 15, 2022 at 05:22:15AM +0200, Mika Penttilä wrote: > Hi Jason and thanks for your comments.. > > On 14.3.2022 20.24, Jason Gunthorpe wrote: > > On Fri, Mar 11, 2022 at 05:30:50AM +0200, mpenttil@redhat.com wrote: > > > From: Mika Penttilä > > > > > > HMM selftests use an in-kernel pseudo device to emulate device private > > > memory. The pseudo device registers a major device range for two pseudo > > > device instances. User space has a script that reads /proc/devices in > > > order to find the assigned major number, and sends that to mknod(1), > > > once for each node. > > > > > > This duplicates a fair amount of boilerplate that misc device can do > > > instead. > > > > > > Change this to use misc device, which makes the device node names appear > > > for us. This also enables udev-like processing if desired. > > > > This is borderline the wrong way to use misc devices, they should > > never be embedded into other structs like this. It works out here > > because they are eventually only placed in a static array, but still > > it is a generally bad pattern to see. > > Could you elaborate on this one? We have many in-tree usages of the same > pattern, like: The kernel is full of bugs > drivers/video/fbdev/pxa3xx-gcu.c ie this is broken because it allocates like this: priv = devm_kzalloc(dev, sizeof(struct pxa3xx_gcu_priv), GFP_KERNEL); if (!priv) return -ENOMEM; And free's via devm: static int pxa3xx_gcu_remove(struct platform_device *pdev) { struct pxa3xx_gcu_priv *priv = platform_get_drvdata(pdev); misc_deregister(&priv->misc_dev); return 0; } But this will UAF if it races fops open with misc_desregister. Proper use of cdevs with proper struct devices prevent this bug. > You mention "placed in a static array", are you seeing a potential lifetime > issue or what? Many of the examples above embed miscdevice in a dynamically > allocated object also. > > The file object's private_data holds a pointer to the miscdevice, and > fops_get() pins the module. So freeing the objects miscdevice is embedded in > at module_exit time should be fine. But, as you said, in this case the > miscdevices are statically allocated, so that shouldn't be an issue > either. Correct, it is OK here because the module refcounts prevent the miscdevice memory from being freed, the above cases with dynamic allocations do not have that protection and are wrong. This is why I don't care for the pattern of putting misc devices inside other structs, it suggests this is perhaps generally safe but it is not. > I think using cdev_add ends up in the same results in device_* api > sense. Nope, everything works right once you use cdev_device_add on a properly registered struct device. > miscdevice acting like a mux at a higher abstraction level simplifies the > code. It does avoid the extra struct device, but at the cost of broken memory lifetime Jason