xen/xenbus: handle potential dangling pointer issue in xen_pcibk_xenbus_probe
PATCH 1/1 xen/xenbus: handle potential dangling pointer issue in xen_pcibk_xenbus_probe
From: Yunseong Kim <[email protected]>
If 'xen_pcibk_init_devices()' fails. This ensures that 'pdev->xdev' does not point to 'xdev' when 'pdev' is freed.
--- drivers/xen/xen-pciback/xenbus.c | 1 + 1 file changed, 1 insertion(+)
drivers/xen/xen-pciback/xenbus.c +1 -0
@@ -54,6 +54,7 @@ static struct xen_pcibk_device *alloc_pdev(struct xenbus_device *xdev)INIT_WORK (&pdev->op_work, xen_pcibk_do_op);if (xen_pcibk_init_devices (pdev)) { + pdev->xdev =NULL ;kfree (pdev); pdev =NULL ; }
Signature · 1 line
2.34.1
Quoted 15 lines · On 29.05.24 14:22, [email protected] wrote:
From: Yunseong Kim <[email protected]>
If '
xen_pcibk_init_devices()' fails. This ensures that 'pdev->xdev' does not point to 'xdev' when 'pdev' is freed.Signed-off-by Yunseong Kim <[email protected]>--- drivers/xen/xen-pciback/xenbus.c | 1 + 1 file changed, 1 insertion(+)drivers/xen/xen-pciback/xenbus.c +0
-0
drivers/xen/xen-pciback/xenbus.c static struct xen_pcibk_device *alloc_pdev(struct xenbus_device *xdev) · line 60
Quoted 7 lines
drivers/xen/xen-pciback/xenbus.c +1
-0@@ -54,6 +54,7 @@ static struct xen_pcibk_device *alloc_pdev(struct xenbus_device *xdev)INIT_WORK (&pdev->op_work, xen_pcibk_do_op);if (xen_pcibk_init_devices (pdev)) { + pdev->xdev =NULL ;kfree (pdev); pdev =NULL ; }
NAK.
This doesn't make any sense, as pdev is freed.
Juergen
Quoted 27 lines · Jürgen Groß
Quoted 16 lines · On 29.05.24 14:22, [email protected] wrote:
From: Yunseong Kim <[email protected]>
If '
xen_pcibk_init_devices()' fails. This ensures that 'pdev->xdev' does not point to 'xdev' when 'pdev' is freed.Signed-off-by Yunseong Kim <[email protected]>---
drivers/xen/xen-pciback/xenbus.c| 1 +1 file changed, 1 insertion(+)drivers/xen/xen-pciback/xenbus.c +0
-0drivers/xen/xen-pciback/xenbus.c static struct xen_pcibk_device *alloc_pdev(struct · line 60
Quoted 7 lines
drivers/xen/xen-pciback/xenbus.c +1
-0@@ -54,6 +54,7 @@ static struct xen_pcibk_device *alloc_pdev(struct xenbus_device *xdev) INIT_WORK(&pdev->op_work, xen_pcibk_do_op); if (xen_pcibk_init_devices(pdev)) { + pdev->xdev = NULL; kfree(pdev); pdev = NULL; }NAK.
Quoted 4 lines
This doesn't make any sense, as pdev is freed.
Juergen
Thanks for the code review Juergen!
I don't know if it's a big deal, but I think that if pdev->xdev don't replace it with NULL, 'struct xenbus_device *xdev' address value stays in the kmalloc cached memory.
And most of the kernel code I've seen does this (even when it's 'free_pdev()'), so I sent a patch.
In the very unlikely event that. I was thinking if someone look at the value without initializing the kmalloced memory, someone might see the 'xenbus_device *xdev address'.
Warm Regards,
Yunseong Kim