| Message ID | 20220502181742.1402826-4-frowand.list@gmail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | of: overlay: unittest: fix overlay notify error, add overlay notifier tests, fix bug revealed by new unittests | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Mon, 02 May 2022 13:17:42 -0500, frowand.list@gmail.com wrote: > From: Frank Rowand <frank.rowand@sony.com> > > 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: Frank Rowand <frank.rowand@sony.com> > --- > Changes since version 1: > - patch 1/1 v1 did not apply on Rob's dt/next branch, rebase on top of > 5f756a2eaa44 of: overlay: do not break notify on NOTIFY_{OK|STOP} > > Output of the new overlay notifier unittests, as filtered by > scripts/dtc/of_unittest_expect: > > ### dt-test ### pass of_unittest_overlay_notify():2825 > ok OF: overlay: overlay changeset pre-apply notifier error -16, target: /testcase-data/overlay-node/test-bus > ### dt-test ### pass of_unittest_overlay_notify():2846 > ### dt-test ### pass of_unittest_overlay_notify():2851 > ok OF: overlay: overlay changeset post-apply notifier error -17, target: /testcase-data/overlay-node/test-bus > ### dt-test ### pass of_unittest_overlay_notify():2857 > ### dt-test ### pass of_unittest_overlay_notify():2862 > ### dt-test ### pass of_unittest_overlay_notify():2866 > ### dt-test ### pass of_unittest_overlay_notify():2872 > ### dt-test ### pass of_unittest_overlay_notify():2875 > ok OF: overlay: overlay changeset pre-remove notifier error -18, target: /testcase-data/overlay-node/test-bus > ### dt-test ### pass of_unittest_overlay_notify():2886 > ### dt-test ### pass of_unittest_overlay_notify():2894 > ### dt-test ### pass of_unittest_overlay_notify():2898 > ### dt-test ### pass of_unittest_overlay_notify():2901 > ok OF: overlay: overlay changeset post-remove notifier error -19, target: /testcase-data/overlay-node/test-bus > ### dt-test ### pass of_unittest_overlay_notify():2908 > ### dt-test ### pass of_unittest_overlay_notify():2915 > ### dt-test ### pass of_unittest_overlay_notify():2920 > ### dt-test ### pass of_unittest_overlay_notify():2932 > > > drivers/of/overlay.c | 29 ++++++++++++++++++++++++++--- > drivers/of/unittest.c | 10 ++++++++-- > 2 files changed, 34 insertions(+), 5 deletions(-) > Applied, thanks!
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index ae5ea5b1079b..4044ddcb02c6 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -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); diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index e28c3df2c4c2..dff55ae09d97 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -2848,7 +2848,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 --- */ @@ -2859,7 +2859,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 --- */