Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fpga: region: add owner module and take its refcount

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.

Fixes: 0fa20cdfcc1f ("fpga: fpga-region: device tree control for FPGA")
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Xu Yilun <yilun.xu@intel.com>
Reviewed-by: Russ Weight <russ.weight@linux.dev>
Signed-off-by: Marco Pagani <marpagan@redhat.com>
Acked-by: Xu Yilun <yilun.xu@intel.com>
Link: https://lore.kernel.org/r/20240419083601.77403-1-marpagan@redhat.com
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>

authored by

Marco Pagani and committed by
Xu Yilun
b7c0e1ec dbca8048

+32 -18
+8 -5
Documentation/driver-api/fpga/fpga-region.rst
··· 46 46 ---------------------------- 47 47 48 48 * struct fpga_region - The FPGA region struct 49 - * struct fpga_region_info - Parameter structure for fpga_region_register_full() 50 - * fpga_region_register_full() - Create and register an FPGA region using the 49 + * struct fpga_region_info - Parameter structure for __fpga_region_register_full() 50 + * __fpga_region_register_full() - Create and register an FPGA region using the 51 51 fpga_region_info structure to provide the full flexibility of options 52 - * fpga_region_register() - Create and register an FPGA region using standard 52 + * __fpga_region_register() - Create and register an FPGA region using standard 53 53 arguments 54 54 * fpga_region_unregister() - Unregister an FPGA region 55 + 56 + Helper macros ``fpga_region_register()`` and ``fpga_region_register_full()`` 57 + automatically set the module that registers the FPGA region as the owner. 55 58 56 59 The FPGA region's probe function will need to get a reference to the FPGA 57 60 Manager it will be using to do the programming. This usually would happen ··· 85 82 :functions: fpga_region_info 86 83 87 84 .. kernel-doc:: drivers/fpga/fpga-region.c 88 - :functions: fpga_region_register_full 85 + :functions: __fpga_region_register_full 89 86 90 87 .. kernel-doc:: drivers/fpga/fpga-region.c 91 - :functions: fpga_region_register 88 + :functions: __fpga_region_register 92 89 93 90 .. kernel-doc:: drivers/fpga/fpga-region.c 94 91 :functions: fpga_region_unregister
+14 -10
drivers/fpga/fpga-region.c
··· 53 53 } 54 54 55 55 get_device(dev); 56 - if (!try_module_get(dev->parent->driver->owner)) { 56 + if (!try_module_get(region->ops_owner)) { 57 57 put_device(dev); 58 58 mutex_unlock(&region->mutex); 59 59 return ERR_PTR(-ENODEV); ··· 75 75 76 76 dev_dbg(dev, "put\n"); 77 77 78 - module_put(dev->parent->driver->owner); 78 + module_put(region->ops_owner); 79 79 put_device(dev); 80 80 mutex_unlock(&region->mutex); 81 81 } ··· 181 181 ATTRIBUTE_GROUPS(fpga_region); 182 182 183 183 /** 184 - * fpga_region_register_full - create and register an FPGA Region device 184 + * __fpga_region_register_full - create and register an FPGA Region device 185 185 * @parent: device parent 186 186 * @info: parameters for FPGA Region 187 + * @owner: module containing the get_bridges function 187 188 * 188 189 * Return: struct fpga_region or ERR_PTR() 189 190 */ 190 191 struct fpga_region * 191 - fpga_region_register_full(struct device *parent, const struct fpga_region_info *info) 192 + __fpga_region_register_full(struct device *parent, const struct fpga_region_info *info, 193 + struct module *owner) 192 194 { 193 195 struct fpga_region *region; 194 196 int id, ret = 0; ··· 215 213 region->compat_id = info->compat_id; 216 214 region->priv = info->priv; 217 215 region->get_bridges = info->get_bridges; 216 + region->ops_owner = owner; 218 217 219 218 mutex_init(&region->mutex); 220 219 INIT_LIST_HEAD(&region->bridge_list); ··· 244 241 245 242 return ERR_PTR(ret); 246 243 } 247 - EXPORT_SYMBOL_GPL(fpga_region_register_full); 244 + EXPORT_SYMBOL_GPL(__fpga_region_register_full); 248 245 249 246 /** 250 - * fpga_region_register - create and register an FPGA Region device 247 + * __fpga_region_register - create and register an FPGA Region device 251 248 * @parent: device parent 252 249 * @mgr: manager that programs this region 253 250 * @get_bridges: optional function to get bridges to a list 251 + * @owner: module containing the get_bridges function 254 252 * 255 253 * This simple version of the register function should be sufficient for most users. 256 254 * The fpga_region_register_full() function is available for users that need to ··· 260 256 * Return: struct fpga_region or ERR_PTR() 261 257 */ 262 258 struct fpga_region * 263 - fpga_region_register(struct device *parent, struct fpga_manager *mgr, 264 - int (*get_bridges)(struct fpga_region *)) 259 + __fpga_region_register(struct device *parent, struct fpga_manager *mgr, 260 + int (*get_bridges)(struct fpga_region *), struct module *owner) 265 261 { 266 262 struct fpga_region_info info = { 0 }; 267 263 268 264 info.mgr = mgr; 269 265 info.get_bridges = get_bridges; 270 266 271 - return fpga_region_register_full(parent, &info); 267 + return __fpga_region_register_full(parent, &info, owner); 272 268 } 273 - EXPORT_SYMBOL_GPL(fpga_region_register); 269 + EXPORT_SYMBOL_GPL(__fpga_region_register); 274 270 275 271 /** 276 272 * fpga_region_unregister - unregister an FPGA region
+10 -3
include/linux/fpga/fpga-region.h
··· 36 36 * @mgr: FPGA manager 37 37 * @info: FPGA image info 38 38 * @compat_id: FPGA region id for compatibility check. 39 + * @ops_owner: module containing the get_bridges function 39 40 * @priv: private data 40 41 * @get_bridges: optional function to get bridges to a list 41 42 */ ··· 47 46 struct fpga_manager *mgr; 48 47 struct fpga_image_info *info; 49 48 struct fpga_compat_id *compat_id; 49 + struct module *ops_owner; 50 50 void *priv; 51 51 int (*get_bridges)(struct fpga_region *region); 52 52 }; ··· 60 58 61 59 int fpga_region_program_fpga(struct fpga_region *region); 62 60 61 + #define fpga_region_register_full(parent, info) \ 62 + __fpga_region_register_full(parent, info, THIS_MODULE) 63 63 struct fpga_region * 64 - fpga_region_register_full(struct device *parent, const struct fpga_region_info *info); 64 + __fpga_region_register_full(struct device *parent, const struct fpga_region_info *info, 65 + struct module *owner); 65 66 67 + #define fpga_region_register(parent, mgr, get_bridges) \ 68 + __fpga_region_register(parent, mgr, get_bridges, THIS_MODULE) 66 69 struct fpga_region * 67 - fpga_region_register(struct device *parent, struct fpga_manager *mgr, 68 - int (*get_bridges)(struct fpga_region *)); 70 + __fpga_region_register(struct device *parent, struct fpga_manager *mgr, 71 + int (*get_bridges)(struct fpga_region *), struct module *owner); 69 72 void fpga_region_unregister(struct fpga_region *region); 70 73 71 74 #endif /* _FPGA_REGION_H */