* question about drivers/phy/renesas/phy-rcar-gen2.c @ 2019-02-03 7:02 Julia Lawall 2019-02-26 8:54 ` Yoshihiro Shimoda 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2019-02-03 7:02 UTC (permalink / raw) To: Kishon Vijay Abraham I; +Cc: Yoshihiro Shimoda, linux-kernel Hello, I was wondering whether phy-rcar-gen2.c would use dynamically allocated device nodes? If so, it looks like the following code could cause a use-after-free, due to not incrementing th reference count: for_each_child_of_node(dev->of_node, np) { struct rcar_gen2_channel *channel = drv->channels + i; u32 channel_num; int error, n; channel->of_node = np; ... } On the other hand, if the reference cound it incrememnted, preventing memory leaks in the case where the probe function fails would entail some complex rewriting of the code, so I thought it would be better to ask first. thanks, julia ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: question about drivers/phy/renesas/phy-rcar-gen2.c 2019-02-03 7:02 question about drivers/phy/renesas/phy-rcar-gen2.c Julia Lawall @ 2019-02-26 8:54 ` Yoshihiro Shimoda 2019-02-26 9:01 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Yoshihiro Shimoda @ 2019-02-26 8:54 UTC (permalink / raw) To: Julia Lawall Cc: linux-kernel, Kishon Vijay Abraham I, Sergei Shtylyov (sergei.shtylyov@cogentembedded.com) Hello, (Sergei made this code, so I added his email as CC) I'm sorry for the delayed response. > From: Julia Lawall, Sent: Sunday, February 3, 2019 4:03 PM > > Hello, > > I was wondering whether phy-rcar-gen2.c would use dynamically allocated > device nodes? I'm sorry, but what is "dynamically allocated device nodes"? Best regards, Yoshihiro Shimoda > If so, it looks like the following code could cause a > use-after-free, due to not incrementing th reference count: > > for_each_child_of_node(dev->of_node, np) { > struct rcar_gen2_channel *channel = drv->channels + i; > u32 channel_num; > int error, n; > > channel->of_node = np; > ... > } > > On the other hand, if the reference cound it incrememnted, preventing > memory leaks in the case where the probe function fails would entail some > complex rewriting of the code, so I thought it would be better to ask > first. > > thanks, > julia ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: question about drivers/phy/renesas/phy-rcar-gen2.c 2019-02-26 8:54 ` Yoshihiro Shimoda @ 2019-02-26 9:01 ` Julia Lawall 2019-02-27 1:06 ` Yoshihiro Shimoda 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2019-02-26 9:01 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: Julia Lawall, linux-kernel, Kishon Vijay Abraham I, Sergei Shtylyov (sergei.shtylyov@cogentembedded.com) On Tue, 26 Feb 2019, Yoshihiro Shimoda wrote: > Hello, > (Sergei made this code, so I added his email as CC) > > I'm sorry for the delayed response. > > > From: Julia Lawall, Sent: Sunday, February 3, 2019 4:03 PM > > > > Hello, > > > > I was wondering whether phy-rcar-gen2.c would use dynamically allocated > > device nodes? > > I'm sorry, but what is "dynamically allocated device nodes"? Device nodes for which there will be a meor leak if one doesn't put of_node_put. julia > > Best regards, > Yoshihiro Shimoda > > > If so, it looks like the following code could cause a > > use-after-free, due to not incrementing th reference count: > > > > for_each_child_of_node(dev->of_node, np) { > > struct rcar_gen2_channel *channel = drv->channels + i; > > u32 channel_num; > > int error, n; > > > > channel->of_node = np; > > ... > > } > > > > On the other hand, if the reference cound it incrememnted, preventing > > memory leaks in the case where the probe function fails would entail some > > complex rewriting of the code, so I thought it would be better to ask > > first. > > > > thanks, > > julia > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: question about drivers/phy/renesas/phy-rcar-gen2.c 2019-02-26 9:01 ` Julia Lawall @ 2019-02-27 1:06 ` Yoshihiro Shimoda 2019-02-27 6:08 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Yoshihiro Shimoda @ 2019-02-27 1:06 UTC (permalink / raw) To: Julia Lawall Cc: linux-kernel, Kishon Vijay Abraham I, Sergei Shtylyov (sergei.shtylyov@cogentembedded.com) Hello, > From: Julia Lawall, Sent: Tuesday, February 26, 2019 6:01 PM > > On Tue, 26 Feb 2019, Yoshihiro Shimoda wrote: > > > Hello, > > (Sergei made this code, so I added his email as CC) > > > > I'm sorry for the delayed response. > > > > > From: Julia Lawall, Sent: Sunday, February 3, 2019 4:03 PM > > > > > > Hello, > > > > > > I was wondering whether phy-rcar-gen2.c would use dynamically allocated > > > device nodes? > > > > I'm sorry, but what is "dynamically allocated device nodes"? > > Device nodes for which there will be a meor leak if one doesn't put > of_node_put. Thank you. I understood it. > julia > > > > > Best regards, > > Yoshihiro Shimoda > > > > > If so, it looks like the following code could cause a > > > use-after-free, due to not incrementing th reference count: > > > > > > for_each_child_of_node(dev->of_node, np) { > > > struct rcar_gen2_channel *channel = drv->channels + i; > > > u32 channel_num; > > > int error, n; > > > > > > channel->of_node = np; IIUC, since the channel->of_node will be used for comparing the pointer in rcar_gen2_phy_xlate(), it is not use-after-free. However, the for_each_child_of_node() in rcar_gen2_phy_probe() will return without of_put_node() at error paths. So, I'll submit a bugfix patch later. Thank you very much for your report! Best regards, Yoshihiro Shimoda > > > ... > > > } > > > > > > On the other hand, if the reference cound it incrememnted, preventing > > > memory leaks in the case where the probe function fails would entail some > > > complex rewriting of the code, so I thought it would be better to ask > > > first. > > > > > > thanks, > > > julia > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: question about drivers/phy/renesas/phy-rcar-gen2.c 2019-02-27 1:06 ` Yoshihiro Shimoda @ 2019-02-27 6:08 ` Julia Lawall 0 siblings, 0 replies; 5+ messages in thread From: Julia Lawall @ 2019-02-27 6:08 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: linux-kernel, Kishon Vijay Abraham I, Sergei Shtylyov (sergei.shtylyov@cogentembedded.com) On Wed, 27 Feb 2019, Yoshihiro Shimoda wrote: > Hello, > > > From: Julia Lawall, Sent: Tuesday, February 26, 2019 6:01 PM > > > > On Tue, 26 Feb 2019, Yoshihiro Shimoda wrote: > > > > > Hello, > > > (Sergei made this code, so I added his email as CC) > > > > > > I'm sorry for the delayed response. > > > > > > > From: Julia Lawall, Sent: Sunday, February 3, 2019 4:03 PM > > > > > > > > Hello, > > > > > > > > I was wondering whether phy-rcar-gen2.c would use dynamically allocated > > > > device nodes? > > > > > > I'm sorry, but what is "dynamically allocated device nodes"? > > > > Device nodes for which there will be a meor leak if one doesn't put > > of_node_put. > > Thank you. I understood it. > > > julia > > > > > > > > Best regards, > > > Yoshihiro Shimoda > > > > > > > If so, it looks like the following code could cause a > > > > use-after-free, due to not incrementing th reference count: > > > > > > > > for_each_child_of_node(dev->of_node, np) { > > > > struct rcar_gen2_channel *channel = drv->channels + i; > > > > u32 channel_num; > > > > int error, n; > > > > > > > > channel->of_node = np; > > IIUC, since the channel->of_node will be used for comparing the pointer > in rcar_gen2_phy_xlate(), it is not use-after-free. > However, the for_each_child_of_node() in rcar_gen2_phy_probe() will return > without of_put_node() at error paths. So, I'll submit a bugfix patch later. > > Thank you very much for your report! Thanks for taking care of it. julia > > Best regards, > Yoshihiro Shimoda > > > > > ... > > > > } > > > > > > > > On the other hand, if the reference cound it incrememnted, preventing > > > > memory leaks in the case where the probe function fails would entail some > > > > complex rewriting of the code, so I thought it would be better to ask > > > > first. > > > > > > > > thanks, > > > > julia > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-27 6:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-03 7:02 question about drivers/phy/renesas/phy-rcar-gen2.c Julia Lawall 2019-02-26 8:54 ` Yoshihiro Shimoda 2019-02-26 9:01 ` Julia Lawall 2019-02-27 1:06 ` Yoshihiro Shimoda 2019-02-27 6:08 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).