On 21/04/2022 00:54, Michael Ellerman wrote:
Hangyu Hua <hbh...@gmail.com> writes:
info_release() will be called in device_unregister() when info->dev's
reference count is 0. So there is no need to call ocxl_afu_put() and
kfree() again.
Double frees are often exploitable. But it looks to me like this error
path is not easily reachable by an attacker.
ocxl_file_register_afu() is only called from ocxl_probe(), and we only
go to err_unregister if the sysfs or cdev initialisation fails, which
should only happen if we hit ENOMEM, or we have a duplicate device which
would be a device-tree/hardware error. But maybe Fred can check more
closely, I don't know the driver that well.
The linux devices built here are based on what is parsed on the physical
devices. Those could be FPGAs but updating the FPGA image requires root
privilege. In any case, duplicate AFU names are possible, that's why the
driver adds an index (the afu->config.idx part of the name) to the linux
device name. So we would need to mess that up in the driver as well to
have a duplicate device name.
So I would agree the double free is hard to hit.
mpe: I think this patch can be taken as is. The "beautification" I
talked about is just that and I don't intend to work on it except if
something else shows up.
Fred
cheers
Fix this by adding free_minor() and return to err_unregister error path.
Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend &
frontend")
Signed-off-by: Hangyu Hua <hbh...@gmail.com>
---
drivers/misc/ocxl/file.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d881f5e40ad9..6777c419a8da 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -556,7 +556,9 @@ int ocxl_file_register_afu(struct ocxl_afu *afu)
err_unregister:
ocxl_sysfs_unregister_afu(info); // safe to call even if register failed
+ free_minor(info);
device_unregister(&info->dev);
+ return rc;
err_put:
ocxl_afu_put(afu);
free_minor(info);
--
2.25.1
