Commit 421f4d14 authored by Frank Rowand's avatar Frank Rowand Committed by Rob Herring
Browse files

of: overlay: do not free changeset when of_overlay_apply returns error 



New unittests for overlay notifiers reveal a memory leak in
of_overlay_apply() when a notifier returns an error for action
OF_OVERLAY_POST_APPLY.  The pr_err() message is:

   OF: ERROR: memory leak, expected refcount 1 instead of 3,
   of_node_get()/of_node_put() unbalanced - destroy cset entry: attach
   overlay node /testcase-data/overlay-node/test-bus/test-unittest17

Change the error path to no longer call free_overlay_changeset(),
and document that the caller of of_overlay_fdt_apply() may choose
to remove the overlay.

Update the unittest that triggered the error to expect the changed
return values and to call of_overlay_remove().

Signed-off-by: default avatarFrank Rowand <frank.rowand@sony.com>
Signed-off-by: default avatarRob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20220502181742.1402826-4-frowand.list@gmail.com
parent 992b0dc5

drivers/of/overlay.c

+26 −3
Original line number Diff line number Diff line
@@ -952,6 +952,25 @@ static int of_overlay_apply(struct overlay_changeset *ovcs) 
	return ret;

}



/*

 * of_overlay_fdt_apply() - Create and apply an overlay changeset

 * @overlay_fdt:	pointer to overlay FDT

 * @overlay_fdt_size:	number of bytes in @overlay_fdt

 * @ret_ovcs_id:	pointer for returning created changeset id

 *

 * Creates and applies an overlay changeset.

 *

 * See of_overlay_apply() for important behavior information.

 *

 * Return: 0 on success, or a negative error number.  *@ret_ovcs_id is set to

 * the value of overlay changeset id, which can be passed to of_overlay_remove()

 * to remove the overlay.

 *

 * On error return, the changeset may be partially applied.  This is especially

 * likely if an OF_OVERLAY_POST_APPLY notifier returns an error.  In this case

 * the caller should call of_overlay_remove() with the value in *@ret_ovcs_id.

 */



int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,

			 int *ret_ovcs_id)

{
@@ -1019,15 +1038,19 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, 
	ovcs->overlay_mem = overlay_mem;



	ret = of_overlay_apply(ovcs);

	if (ret < 0)

		goto err_free_ovcs;

	/*

	 * If of_overlay_apply() error, calling free_overlay_changeset() may

	 * result in a memory leak if the apply partly succeeded, so do NOT

	 * goto err_free_ovcs.  Instead, the caller of of_overlay_fdt_apply()

	 * can call of_overlay_remove();

	 */



	mutex_unlock(&of_mutex);

	of_overlay_mutex_unlock();



	*ret_ovcs_id = ovcs->id;



	return 0;

	return ret;



err_free_ovcs:

	free_overlay_changeset(ovcs);

drivers/of/unittest.c

+8 −2
Original line number Diff line number Diff line
@@ -2845,7 +2845,7 @@ static void __init of_unittest_overlay_notify(void) 


	EXPECT_END(KERN_INFO, "OF: overlay: overlay changeset pre-apply notifier error -16, target: /testcase-data/overlay-node/test-bus");



	unittest(!ovcs_id, "ovcs_id created for overlay_16\n");

	unittest(ovcs_id, "ovcs_id not created for overlay_16\n");



	/* ---  overlay 17  --- */
@@ -2856,7 +2856,13 @@ static void __init of_unittest_overlay_notify(void) 


	EXPECT_END(KERN_INFO, "OF: overlay: overlay changeset post-apply notifier error -17, target: /testcase-data/overlay-node/test-bus");



	unittest(!ovcs_id, "ovcs_id created for overlay_17\n");

	unittest(ovcs_id, "ovcs_id not created for overlay_17\n");



	if (ovcs_id) {

		ret = of_overlay_remove(&ovcs_id);

		unittest(!ret,

			"overlay_17 of_overlay_remove(), ret = %d\n", ret);

	}



	/* ---  overlay 18  --- */