| Message ID | 20220601042514.61780-1-linmq006@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v4l: xilinx-vipp: Fix refcount leak in xvip_graph_dma_init | expand |
Hi Lin, Thank you for the patch. On Wed, Jun 01, 2022 at 08:25:14AM +0400, Miaoqian Lin wrote: > of_get_child_by_name() returns a node pointer with refcount > incremented, we should use of_node_put() on it when not need anymore. > Add missing of_node_put() to avoid refcount leak. > > Fixes: df3305156f98 ("[media] v4l: xilinx: Add Xilinx Video IP core") > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > --- > drivers/media/platform/xilinx/xilinx-vipp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c > index f34f8b077e03..415579b63737 100644 > --- a/drivers/media/platform/xilinx/xilinx-vipp.c > +++ b/drivers/media/platform/xilinx/xilinx-vipp.c > @@ -483,10 +483,12 @@ static int xvip_graph_dma_init(struct xvip_composite_device *xdev) > ret = xvip_graph_dma_init_one(xdev, port); > if (ret < 0) { > of_node_put(port); > + of_node_put(ports); > return ret; > } > } > > + of_node_put(ports); We could possibly simplify this a little bit by initializing ret to 0 when declaring it, replacing the "return ret" above by a break, and returning ret below. This would remove the need for the first of_node_put(ports) call above. If you think that's a good idea I can make that change when applying this patch to my tree, otherwise I'll take it as-is. In either case, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > return 0; > } >
Hi, Laurent On 2022/6/1 15:34, Laurent Pinchart wrote: > Hi Lin, > > Thank you for the patch. > > On Wed, Jun 01, 2022 at 08:25:14AM +0400, Miaoqian Lin wrote: >> of_get_child_by_name() returns a node pointer with refcount >> incremented, we should use of_node_put() on it when not need anymore. >> Add missing of_node_put() to avoid refcount leak. >> >> Fixes: df3305156f98 ("[media] v4l: xilinx: Add Xilinx Video IP core") >> Signed-off-by: Miaoqian Lin <linmq006@gmail.com> >> --- >> drivers/media/platform/xilinx/xilinx-vipp.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c >> index f34f8b077e03..415579b63737 100644 >> --- a/drivers/media/platform/xilinx/xilinx-vipp.c >> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c >> @@ -483,10 +483,12 @@ static int xvip_graph_dma_init(struct xvip_composite_device *xdev) >> ret = xvip_graph_dma_init_one(xdev, port); >> if (ret < 0) { >> of_node_put(port); >> + of_node_put(ports); >> return ret; >> } >> } >> >> + of_node_put(ports); > We could possibly simplify this a little bit by initializing ret to 0 > when declaring it, replacing the "return ret" above by a break, and > returning ret below. This would remove the need for the first > of_node_put(ports) call above. > > If you think that's a good idea I can make that change when applying > this patch to my tree, otherwise I'll take it as-is. Thanks for your review, I think your idea is good, we can do that to simplify code. > In either case, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> return 0; >> } >>
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c index f34f8b077e03..415579b63737 100644 --- a/drivers/media/platform/xilinx/xilinx-vipp.c +++ b/drivers/media/platform/xilinx/xilinx-vipp.c @@ -483,10 +483,12 @@ static int xvip_graph_dma_init(struct xvip_composite_device *xdev) ret = xvip_graph_dma_init_one(xdev, port); if (ret < 0) { of_node_put(port); + of_node_put(ports); return ret; } } + of_node_put(ports); return 0; }
of_get_child_by_name() returns a node pointer with refcount incremented, we should use of_node_put() on it when not need anymore. Add missing of_node_put() to avoid refcount leak. Fixes: df3305156f98 ("[media] v4l: xilinx: Add Xilinx Video IP core") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> --- drivers/media/platform/xilinx/xilinx-vipp.c | 2 ++ 1 file changed, 2 insertions(+)