[PATCH 2/2] peci: Fix use-after-free in remove_store()

[Suyuan Wang]

remove_store() doesn't hold a reference to the device across

    if (res && device_remove_file_self(dev, attr))
            peci_device_destroy(device);

Once device_remove_file_self() has dropped the sysfs active protection,
nothing keeps the device alive. peci_device_destroy() then does

    device_unregister(&device->dev);
    device->deleted = true;

and if device_unregister() dropped the last reference, the second line
writes to freed memory. So writing 1 to the "remove" attribute usually
ends in a use-after-free. A controller removal running at the same time
can also destroy and free the device first, and then
peci_device_destroy() reads device->deleted from freed memory.

Take a reference before device_remove_file_self() and drop it at the
end, the same way sdev_store_delete() does in SCSI. The deleted flag
still makes sure that only one of the two paths unregisters the device.

Found with a static analysis tool and checked by hand.

Fixes: 42bed52b2e9c ("peci: Add sysfs interface for PECI bus")
Cc: stable@xxxxxxxxxxxxxxx
Co-developed-by: Kaixuan Li <kaixuan.li@xxxxxxxxxx>
Signed-off-by: Kaixuan Li <kaixuan.li@xxxxxxxxxx>
Signed-off-by: Suyuan Wang <stevewong@xxxxxxxxxxx>
---
Compile tested only. The race with controller removal comes from
reading the code, I haven't reproduced it.

 drivers/peci/sysfs.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/peci/sysfs.c b/drivers/peci/sysfs.c
index c04244075794..4b1c8b91a3cc 100644
--- a/drivers/peci/sysfs.c
+++ b/drivers/peci/sysfs.c
@@ -60,8 +60,19 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	if (res && device_remove_file_self(dev, attr))
+	if (!res)
+		return count;
+
+	/*
+	 * device_remove_file_self() drops the sysfs active protection, after
+	 * which the device can be unregistered and freed, either by
+	 * peci_device_destroy() below or by a concurrent controller removal.
+	 * Hold a reference until we are done with it.
+	 */
+	get_device(dev);
+	if (device_remove_file_self(dev, attr))
 		peci_device_destroy(device);
+	put_device(dev);
 
 	return count;
 }
-- 
2.34.1