From nobody Sun Feb 8 09:32:34 2026 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BA8F75024E for ; Wed, 27 Mar 2024 16:01:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711555307; cv=none; b=qXxIaw76IQ21Bs7HqAwdyiklx/exRGM6xqC0BuH6XI7P8ISs6PvfnMnbno6qmjBMChLWzb/gY/yU2aH9zjN82xXTB/z8pHDLw4jaYXWtHpt78LEshXrpJluCWoL8xGrTRlH1WbG045T/ZiJeAKmqSoLCv9B6SaBORBeg48f6lhk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711555307; c=relaxed/simple; bh=sCzNGZzazf+KJukAXuHWhrhnZJ5Xtoa9qDS9guDzCUA=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=ftdnYB3TLY44EwvjLPpVxoeZOXj96Qr9Vm2/jmNFcH1RI7OacYLjPoITwOvwHbU6NOCUiA1ISW7gTRxaSh+ojGXP1B8MWcIN+p7tOWLLxry/GEXrQ/OzoQ18cM1DvHRKEc8nn7n2x3A5HwZvgQi1HaoXhYk1DE2Xk2LpJ58HDe0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=cb96jY4f; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="cb96jY4f" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711555304; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=sV/lZgwkhea3I+lgwlHmyuTZ7NXsG6CjPnVg+nnNBQ8=; b=cb96jY4fLWHzY+GVMZec0xKRqvvvuyhZhBR7SSQm5DOxhOJuxJPuGRtvk6KQE4ggHwV+iH MG8xNShlpggahuxospxkBSW0D7WPASDROq1snsRm6fo2WVUOzCWCfOz07/51TqsHfeIwmI V2MxlpNaqa9nMNw9HiR2EWlukzj6CnQ= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-687-1Kb6HNrCM7GadN3r-yznEQ-1; Wed, 27 Mar 2024 12:01:42 -0400 X-MC-Unique: 1Kb6HNrCM7GadN3r-yznEQ-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-3418f237c0bso3514308f8f.3 for ; Wed, 27 Mar 2024 09:01:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711555302; x=1712160102; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=sV/lZgwkhea3I+lgwlHmyuTZ7NXsG6CjPnVg+nnNBQ8=; b=kYBC/fec05al34E6lZFIriOGbg44KM7h8G8/lp4QR6/4Vu8bgxaU3KQOM4NsVOFwkb NH5HP+T2TOMSnafYVBla1xdp5X0A6Wz1JQqS+6XM2/UPwYSkWoWkllzIUTSNoVWhEzr9 puDTQrQxhEz7hB4KglhPgHxeZHqlZe6k0g71/oiGRsFvetZHouUAzcRYvB5tZ4lxvtUN NOR+NKKHLB4ZQDjhlMRBPSevX/EfiqZfw07GY0VL4UulkGDw+5rjDNYgkJi4Yn/RZ1Mv IxUyo/I7qwA+RIwTG4yhmRCPIHO9mOo60N4FAdxVncHtheDi2hkFM1xPUgT13Yf2ysHr tXig== X-Forwarded-Encrypted: i=1; AJvYcCWDizb4z18SMXaAckY6YwCl7L39bt+mcx/ROmaqfqYcsUMejJ/y/yqdlavOnXdAe789dN1l4gVoCLg73mEj6J8GO2K8lowUB/gCdvg+ X-Gm-Message-State: AOJu0YxOnxdfzFHrD6ZMrw2aOTLxLjE9q3DpDBXVcV2zZ8o6DqPVNNmQ FxWvZ1D8NUh8WDOl0tVxafUfebizvLBJ8a/cfxOqYBUoGEVzzGe1tRKx2CIPKzP9Znf695xJ5cZ 73MZcb/HbeAH2Kr14SvYScD1CfqOdg12XZuAqOcAQz+V9+HBKK4QfYaVPlDfY X-Received: by 2002:a05:6000:188a:b0:33e:8b95:b351 with SMTP id a10-20020a056000188a00b0033e8b95b351mr325961wri.9.1711555301600; Wed, 27 Mar 2024 09:01:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFMHzSy070bVLduzjetNQIXt6kHnR1+LnSFRxpL9zxKrA3nP/E66ofJjxLjiEBU6orzyjofKw== X-Received: by 2002:a05:6000:188a:b0:33e:8b95:b351 with SMTP id a10-20020a056000188a00b0033e8b95b351mr325938wri.9.1711555301136; Wed, 27 Mar 2024 09:01:41 -0700 (PDT) Received: from klayman.redhat.com (net-2-34-30-89.cust.vodafonedsl.it. [2.34.30.89]) by smtp.gmail.com with ESMTPSA id x3-20020adfcc03000000b0033e41e1ad93sm15114709wrh.57.2024.03.27.09.01.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Mar 2024 09:01:40 -0700 (PDT) From: Marco Pagani To: Moritz Fischer , Wu Hao , Xu Yilun , Tom Rix , Jonathan Corbet , Greg Kroah-Hartman , Alan Tull Cc: Marco Pagani , linux-fpga@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] fpga: region: add owner module and take its refcount Date: Wed, 27 Mar 2024 17:00:20 +0100 Message-ID: <20240327160022.202934-1-marpagan@redhat.com> X-Mailer: git-send-email 2.44.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The current implementation of the fpga region assumes that the low-level module registers a driver for the parent device and uses its owner pointer to take the module's refcount. This approach is problematic since it can lead to a null pointer dereference while attempting to get the region during programming if the parent device does not have a driver. To address this problem, add a module owner pointer to the fpga_region struct and use it to take the module's refcount. Modify the functions for registering a region to take an additional owner module parameter and rename them to avoid conflicts. Use the old function names for helper macros that automatically set the module that registers the region as the owner. This ensures compatibility with existing low-level control modules and reduces the chances of registering a region without setting the owner. Also, update the documentation to keep it consistent with the new interface for registering an fpga region. Other changes: unlock the mutex before calling put_device() in fpga_region_put() to avoid potential use after release issues. Fixes: 0fa20cdfcc1f ("fpga: fpga-region: device tree control for FPGA") Suggested-by: Greg Kroah-Hartman Suggested-by: Xu Yilun Signed-off-by: Marco Pagani Reviewed-by: Russ Weight --- v2: - Fixed typo in the documentation sets -> set - Renamed owner pointer get_br_owner -> br_owner --- Documentation/driver-api/fpga/fpga-region.rst | 13 ++++++---- drivers/fpga/fpga-region.c | 26 +++++++++++-------- include/linux/fpga/fpga-region.h | 13 +++++++--- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/= driver-api/fpga/fpga-region.rst index dc55d60a0b4a..77190a5ef330 100644 --- a/Documentation/driver-api/fpga/fpga-region.rst +++ b/Documentation/driver-api/fpga/fpga-region.rst @@ -46,13 +46,16 @@ API to add a new FPGA region ---------------------------- =20 * struct fpga_region - The FPGA region struct -* struct fpga_region_info - Parameter structure for fpga_region_register_f= ull() -* fpga_region_register_full() - Create and register an FPGA region using = the +* struct fpga_region_info - Parameter structure for __fpga_region_register= _full() +* __fpga_region_register_full() - Create and register an FPGA region usin= g the fpga_region_info structure to provide the full flexibility of options -* fpga_region_register() - Create and register an FPGA region using stand= ard +* __fpga_region_register() - Create and register an FPGA region using sta= ndard arguments * fpga_region_unregister() - Unregister an FPGA region =20 +Helper macros ``fpga_region_register()`` and ``fpga_region_register_full()= `` +automatically set the module that registers the FPGA region as the owner. + The FPGA region's probe function will need to get a reference to the FPGA Manager it will be using to do the programming. This usually would happen during the region's probe function. @@ -82,10 +85,10 @@ following APIs to handle building or tearing down that = list. :functions: fpga_region_info =20 .. kernel-doc:: drivers/fpga/fpga-region.c - :functions: fpga_region_register_full + :functions: __fpga_region_register =20 .. kernel-doc:: drivers/fpga/fpga-region.c - :functions: fpga_region_register + :functions: __fpga_region_register_full =20 .. kernel-doc:: drivers/fpga/fpga-region.c :functions: fpga_region_unregister diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index b364a929425c..1beb7415c2dc 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -53,7 +53,7 @@ static struct fpga_region *fpga_region_get(struct fpga_re= gion *region) } =20 get_device(dev); - if (!try_module_get(dev->parent->driver->owner)) { + if (!try_module_get(region->br_owner)) { put_device(dev); mutex_unlock(®ion->mutex); return ERR_PTR(-ENODEV); @@ -75,9 +75,9 @@ static void fpga_region_put(struct fpga_region *region) =20 dev_dbg(dev, "put\n"); =20 - module_put(dev->parent->driver->owner); - put_device(dev); + module_put(region->br_owner); mutex_unlock(®ion->mutex); + put_device(dev); } =20 /** @@ -181,14 +181,16 @@ static struct attribute *fpga_region_attrs[] =3D { ATTRIBUTE_GROUPS(fpga_region); =20 /** - * fpga_region_register_full - create and register an FPGA Region device + * __fpga_region_register_full - create and register an FPGA Region device * @parent: device parent * @info: parameters for FPGA Region + * @owner: owner module containing the get_bridges function * * Return: struct fpga_region or ERR_PTR() */ struct fpga_region * -fpga_region_register_full(struct device *parent, const struct fpga_region_= info *info) +__fpga_region_register_full(struct device *parent, const struct fpga_regio= n_info *info, + struct module *owner) { struct fpga_region *region; int id, ret =3D 0; @@ -213,6 +215,7 @@ fpga_region_register_full(struct device *parent, const = struct fpga_region_info * region->compat_id =3D info->compat_id; region->priv =3D info->priv; region->get_bridges =3D info->get_bridges; + region->br_owner =3D owner; =20 mutex_init(®ion->mutex); INIT_LIST_HEAD(®ion->bridge_list); @@ -241,13 +244,14 @@ fpga_region_register_full(struct device *parent, cons= t struct fpga_region_info * =20 return ERR_PTR(ret); } -EXPORT_SYMBOL_GPL(fpga_region_register_full); +EXPORT_SYMBOL_GPL(__fpga_region_register_full); =20 /** - * fpga_region_register - create and register an FPGA Region device + * __fpga_region_register - create and register an FPGA Region device * @parent: device parent * @mgr: manager that programs this region * @get_bridges: optional function to get bridges to a list + * @owner: owner module containing get_bridges function * * This simple version of the register function should be sufficient for m= ost users. * The fpga_region_register_full() function is available for users that ne= ed to @@ -256,17 +260,17 @@ EXPORT_SYMBOL_GPL(fpga_region_register_full); * Return: struct fpga_region or ERR_PTR() */ struct fpga_region * -fpga_region_register(struct device *parent, struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)) +__fpga_region_register(struct device *parent, struct fpga_manager *mgr, + int (*get_bridges)(struct fpga_region *), struct module *owner) { struct fpga_region_info info =3D { 0 }; =20 info.mgr =3D mgr; info.get_bridges =3D get_bridges; =20 - return fpga_region_register_full(parent, &info); + return __fpga_region_register_full(parent, &info, owner); } -EXPORT_SYMBOL_GPL(fpga_region_register); +EXPORT_SYMBOL_GPL(__fpga_region_register); =20 /** * fpga_region_unregister - unregister an FPGA region diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-reg= ion.h index 9d4d32909340..d175babc3d68 100644 --- a/include/linux/fpga/fpga-region.h +++ b/include/linux/fpga/fpga-region.h @@ -36,6 +36,7 @@ struct fpga_region_info { * @mgr: FPGA manager * @info: FPGA image info * @compat_id: FPGA region id for compatibility check. + * @br_owner: module containing the get_bridges function * @priv: private data * @get_bridges: optional function to get bridges to a list */ @@ -46,6 +47,7 @@ struct fpga_region { struct fpga_manager *mgr; struct fpga_image_info *info; struct fpga_compat_id *compat_id; + struct module *br_owner; void *priv; int (*get_bridges)(struct fpga_region *region); }; @@ -58,12 +60,17 @@ fpga_region_class_find(struct device *start, const void= *data, =20 int fpga_region_program_fpga(struct fpga_region *region); =20 +#define fpga_region_register_full(parent, info) \ + __fpga_region_register_full(parent, info, THIS_MODULE) struct fpga_region * -fpga_region_register_full(struct device *parent, const struct fpga_region_= info *info); +__fpga_region_register_full(struct device *parent, const struct fpga_regio= n_info *info, + struct module *owner); =20 +#define fpga_region_register(parent, mgr, get_bridges) \ + __fpga_region_register(parent, mgr, get_bridges, THIS_MODULE) struct fpga_region * -fpga_region_register(struct device *parent, struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)); +__fpga_region_register(struct device *parent, struct fpga_manager *mgr, + int (*get_bridges)(struct fpga_region *), struct module *owner); void fpga_region_unregister(struct fpga_region *region); =20 #endif /* _FPGA_REGION_H */ base-commit: b1a91ca25f15b6d7b311de4465854a5981dee3d3 --=20 2.44.0