* [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate
@ 2015-04-22 16:14 Ricardo Ribalda Delgado
2015-04-22 16:14 ` [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Ricardo Ribalda Delgado
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-04-22 16:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton,
Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu,
Mike Travis, Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Ricardo Ribalda Delgado
of_platform_depopulate can lead to a kernel error when calling release_resource()
of_platform_depopulate()
of_platform_device_destroy()
platform_device_unregister(platform_device *pdev)
platform_device_del(platform_device *pdev)
for (i = 0; i < pdev->num_resources; i++)
release_resource()
The reason is that it is trying to release a resource that was not allocated
via insert_resource
of_platform_populate()
...
of_device_alloc()
pdev = platform_device_alloc()
# set pdev->resource, similar to platform_device_add_resources()
of_device_add(platform_device *pdev)
# similar to platform_device_add(), but note there's no
# insert_resource() in this path
device_add(&pdev->dev)
This set of patches modifies release_resource to check for
resource->parent==NULL and throw a warning if there is an error.
base/platform has been fixed for an hypothetical condition where parent is
set but the platform is neither MEM or IO.
Then platform_device_alloc has been modified so it supports of and amba
devices.
Finally of_device_add has been modified to use platform_device_add().
v1: https://lkml.org/lkml/2015/4/20/435
v2: https://lkml.org/lkml/2015/4/21/99
https://lkml.org/lkml/2015/4/21/100
Ricardo Ribalda Delgado (4):
kernel/resource: Invalid memory access in __release_resource
base/platform: Only insert MEM and IO resources
base/platform: Continue on insert_resource() error
of/platform: Use platform_device interface
drivers/base/platform.c | 20 ++++++++++++--------
drivers/of/platform.c | 3 ++-
kernel/resource.c | 3 +++
3 files changed, 17 insertions(+), 9 deletions(-)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource 2015-04-22 16:14 [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate Ricardo Ribalda Delgado @ 2015-04-22 16:14 ` Ricardo Ribalda Delgado 2015-04-22 16:47 ` Bjorn Helgaas 2015-04-22 16:14 ` [PATCH v3 2/4] base/platform: Only insert MEM and IO resources Ricardo Ribalda Delgado ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-22 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel, devicetree Cc: Ricardo Ribalda Delgado When a resource is initialized via of_platform_populate. resource->parent is initialized to NULL via kzalloc. (of_platform_populate->of_device_alloc->of_address_to_resource) If of_platform_depopulate is called later, resource->parent is accessed (Offset 0x30 of address 0), causing a kernel error. This patch evaluates resouce->parent before accessing it. If it is not initialized, -EACCESS is returned. Also a WARN is thrown, so the developer can have a hint about what needs to be fixed. Fixes: BUG: unable to handle kernel NULL pointer deference at 0000000000000030 IP: release_resource+0x26/0x90 Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- kernel/resource.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/resource.c b/kernel/resource.c index 90552aa..b7b270f 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -237,6 +237,9 @@ static int __release_resource(struct resource *old) { struct resource *tmp, **p; + if (WARN_ON(!old->parent)) + return -EINVAL; + p = &old->parent->child; for (;;) { tmp = *p; -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource 2015-04-22 16:14 ` [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Ricardo Ribalda Delgado @ 2015-04-22 16:47 ` Bjorn Helgaas [not found] ` <20150422164701.GJ20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 16+ messages in thread From: Bjorn Helgaas @ 2015-04-22 16:47 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel, devicetree On Wed, Apr 22, 2015 at 06:14:18PM +0200, Ricardo Ribalda Delgado wrote: > When a resource is initialized via of_platform_populate. > resource->parent is initialized to NULL via kzalloc. > (of_platform_populate->of_device_alloc->of_address_to_resource) > > If of_platform_depopulate is called later, resource->parent is > accessed (Offset 0x30 of address 0), causing a kernel error. > > This patch evaluates resouce->parent before accessing it. If it > is not initialized, -EACCESS is returned. > > Also a WARN is thrown, so the developer can have a hint about what > needs to be fixed. > > Fixes: > BUG: unable to handle kernel NULL pointer deference at 0000000000000030 > IP: release_resource+0x26/0x90 > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > kernel/resource.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/resource.c b/kernel/resource.c > index 90552aa..b7b270f 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -237,6 +237,9 @@ static int __release_resource(struct resource *old) > { > struct resource *tmp, **p; > > + if (WARN_ON(!old->parent)) > + return -EINVAL; I'm not really a fan of this. The NULL pointer oops is a very good clue all by itself, and it doesn't require any extra code here. > p = &old->parent->child; > for (;;) { > tmp = *p; > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20150422164701.GJ20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource [not found] ` <20150422164701.GJ20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-04-23 8:06 ` Ricardo Ribalda Delgado 0 siblings, 0 replies; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-23 8:06 UTC (permalink / raw) To: Bjorn Helgaas Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, Thierry Reding, LKML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Hi Bjorn On Wed, Apr 22, 2015 at 6:47 PM, Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote: > > I'm not really a fan of this. The NULL pointer oops is a very good clue > all by itself, and it doesn't require any extra code here. It is a pointer to 0x30. For some reason my platform can handle a couple of oops, but if I get a fair amount of them in a short period of time, the whole thing crashes and debugging gets complicated. (no access to dmesg or remote debugging). Therefore this particular bug gave me a bad time. Now that we have located the root of the problem and solved the problem from the root, this particular error will never be hit. I just wanted to save somebody else's time with this patch. All that said, if the other patches are applied, my platform works completely fine. So it is completely your call to accept this or not :) I will resend the patchset without this patch Thanks! -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] base/platform: Only insert MEM and IO resources 2015-04-22 16:14 [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate Ricardo Ribalda Delgado 2015-04-22 16:14 ` [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Ricardo Ribalda Delgado @ 2015-04-22 16:14 ` Ricardo Ribalda Delgado 2015-04-22 16:14 ` [PATCH v3 3/4] base/platform: Continue on insert_resource() error Ricardo Ribalda Delgado 2015-04-22 16:14 ` [PATCH v3 4/4] of/platform: Use platform_device interface Ricardo Ribalda Delgado 3 siblings, 0 replies; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-22 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel, devicetree Cc: Ricardo Ribalda Delgado platform_device_del only checks the type of the resource in order to call release_resource. On the other hand, platform_device_add calls insert_resource for any resource that has a parent. Make both code branches balanced. Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/base/platform.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index ebf034b..6028681 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -341,19 +341,23 @@ int platform_device_add(struct platform_device *pdev) for (i = 0; i < pdev->num_resources; i++) { struct resource *p, *r = &pdev->resource[i]; + unsigned long type = resource_type(r); if (r->name == NULL) r->name = dev_name(&pdev->dev); + if (!(type == IORESOURCE_MEM || type == IORESOURCE_IO)) + continue; + p = r->parent; if (!p) { - if (resource_type(r) == IORESOURCE_MEM) + if (type == IORESOURCE_MEM) p = &iomem_resource; - else if (resource_type(r) == IORESOURCE_IO) + else if (type == IORESOURCE_IO) p = &ioport_resource; } - if (p && insert_resource(p, r)) { + if (insert_resource(p, r)) { dev_err(&pdev->dev, "failed to claim resource %d\n", i); ret = -EBUSY; goto failed; -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] base/platform: Continue on insert_resource() error 2015-04-22 16:14 [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate Ricardo Ribalda Delgado 2015-04-22 16:14 ` [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Ricardo Ribalda Delgado 2015-04-22 16:14 ` [PATCH v3 2/4] base/platform: Only insert MEM and IO resources Ricardo Ribalda Delgado @ 2015-04-22 16:14 ` Ricardo Ribalda Delgado [not found] ` <1429719261-18024-4-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2015-04-22 16:14 ` [PATCH v3 4/4] of/platform: Use platform_device interface Ricardo Ribalda Delgado 3 siblings, 1 reply; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-22 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel, devicetree Cc: Ricardo Ribalda Delgado insert_resource() can fail when the resource added overlaps (partially or fully) with another. Device tree and AMBA devices may contain resources that overlap, so they could not call platform_device_add (Revert "of: use platform_device_add" 02bbde7849e68e193cefaa1885fe0df0f03c9fcd ) On the other hand, device trees are released using platform_device_unregister(). This function calls platform_device_del(), which calls release_resource(), that crashes when the resource has not been added with with insert_resource. This was not an issue when the device tree could not be modified online, but this is not the case anymore. This patch let the flow continue when there is an insert error, after notifying the user with a dev_err(). r->parent is set to NULL, so the device_del knows that the resource was not added, and therefore it should not be released. Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/base/platform.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 6028681..8cce2a3 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -359,8 +359,7 @@ int platform_device_add(struct platform_device *pdev) if (insert_resource(p, r)) { dev_err(&pdev->dev, "failed to claim resource %d\n", i); - ret = -EBUSY; - goto failed; + r->parent = NULL; } } @@ -371,7 +370,6 @@ int platform_device_add(struct platform_device *pdev) if (ret == 0) return ret; - failed: if (pdev->id_auto) { ida_simple_remove(&platform_devid_ida, pdev->id); pdev->id = PLATFORM_DEVID_AUTO; @@ -381,7 +379,8 @@ int platform_device_add(struct platform_device *pdev) struct resource *r = &pdev->resource[i]; unsigned long type = resource_type(r); - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) + if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) && + r->parent) release_resource(r); } @@ -414,7 +413,8 @@ void platform_device_del(struct platform_device *pdev) struct resource *r = &pdev->resource[i]; unsigned long type = resource_type(r); - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) + if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) && + r->parent) release_resource(r); } } -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <1429719261-18024-4-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 3/4] base/platform: Continue on insert_resource() error [not found] ` <1429719261-18024-4-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-04-22 16:44 ` Bjorn Helgaas [not found] ` <20150422164457.GI20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 16+ messages in thread From: Bjorn Helgaas @ 2015-04-22 16:44 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA On Wed, Apr 22, 2015 at 06:14:20PM +0200, Ricardo Ribalda Delgado wrote: > insert_resource() can fail when the resource added overlaps > (partially or fully) with another. > > Device tree and AMBA devices may contain resources that overlap, so they > could not call platform_device_add (Revert "of: use platform_device_add" > 02bbde7849e68e193cefaa1885fe0df0f03c9fcd ) Usual style for referencing a commit is "(see 02bbde7849e6 ('Revert "of: use platform_device_add"'))". > On the other hand, device trees are released using > platform_device_unregister(). This function calls platform_device_del(), > which calls release_resource(), that crashes when the resource has not > been added with with insert_resource. This was not an issue when the > device tree could not be modified online, but this is not the case > anymore. > > This patch let the flow continue when there is an insert error, after > notifying the user with a dev_err(). r->parent is set to NULL, so the > device_del knows that the resource was not added, and therefore it > should not be released. I think you meant platform_device_del()? I don't think device_del() does anything with resources. > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > drivers/base/platform.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 6028681..8cce2a3 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -359,8 +359,7 @@ int platform_device_add(struct platform_device *pdev) > > if (insert_resource(p, r)) { > dev_err(&pdev->dev, "failed to claim resource %d\n", i); Sounds like we should expect to see this message more in the future, after you change of_platform_device_create_pdata() use platform_device_add(). You might want to use insert_resource_conflict() here so you can include information about *why* we failed to claim the resource. And it would be nice to use %pR for both resources. > - ret = -EBUSY; > - goto failed; > + r->parent = NULL; > } > } > > @@ -371,7 +370,6 @@ int platform_device_add(struct platform_device *pdev) > if (ret == 0) > return ret; > > - failed: Might be nice to keep a comment here as a clue that the rest of the function is the failure path. It's a minor style thing, but I would also remove the "err_out" label and return "ret" directly above rather than branching to "err_out". > if (pdev->id_auto) { > ida_simple_remove(&platform_devid_ida, pdev->id); > pdev->id = PLATFORM_DEVID_AUTO; > @@ -381,7 +379,8 @@ int platform_device_add(struct platform_device *pdev) > struct resource *r = &pdev->resource[i]; > unsigned long type = resource_type(r); > > - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) > + if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) && > + r->parent) > release_resource(r); > } > > @@ -414,7 +413,8 @@ void platform_device_del(struct platform_device *pdev) > struct resource *r = &pdev->resource[i]; > unsigned long type = resource_type(r); > > - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) > + if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) && > + r->parent) > release_resource(r); > } > } > -- > 2.1.4 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20150422164457.GI20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v3 3/4] base/platform: Continue on insert_resource() error [not found] ` <20150422164457.GI20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-04-23 7:55 ` Ricardo Ribalda Delgado 2015-04-23 13:26 ` Bjorn Helgaas 0 siblings, 1 reply; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-23 7:55 UTC (permalink / raw) To: Bjorn Helgaas Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, Thierry Reding, LKML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Hi Bjorn: On Wed, Apr 22, 2015 at 6:44 PM, Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote: > Usual style for referencing a commit is "(see 02bbde7849e6 ('Revert "of: > use platform_device_add"'))". Do you make that reference manually or there is a magic git command for printing it in that style? > > Sounds like we should expect to see this message more in the future, after > you change of_platform_device_create_pdata() use platform_device_add(). > You might want to use insert_resource_conflict() here so you can include > information about *why* we failed to claim the resource. And it would be > nice to use %pR for both resources. > ..... >> >> - failed: > > Might be nice to keep a comment here as a clue that the rest of the > function is the failure path. Will prepare a patch with the changes and resend. Thanks! -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] base/platform: Continue on insert_resource() error 2015-04-23 7:55 ` Ricardo Ribalda Delgado @ 2015-04-23 13:26 ` Bjorn Helgaas [not found] ` <20150423132637.GK20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 16+ messages in thread From: Bjorn Helgaas @ 2015-04-23 13:26 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, Thierry Reding, LKML, devicetree@vger.kernel.org On Thu, Apr 23, 2015 at 09:55:13AM +0200, Ricardo Ribalda Delgado wrote: > Hi Bjorn: > > On Wed, Apr 22, 2015 at 6:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > > Usual style for referencing a commit is "(see 02bbde7849e6 ('Revert "of: > > use platform_device_add"'))". > > Do you make that reference manually or there is a magic git command > for printing it in that style? I used to do it mostly by hand, but thanks to your prompting, I fiddled around and came up with this alias: gsr is aliased to `git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n"' Bjorn ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20150423132637.GK20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v3 3/4] base/platform: Continue on insert_resource() error [not found] ` <20150423132637.GK20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-04-23 16:54 ` Thierry Reding 0 siblings, 0 replies; 16+ messages in thread From: Thierry Reding @ 2015-04-23 16:54 UTC (permalink / raw) To: Bjorn Helgaas Cc: Ricardo Ribalda Delgado, Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Jiang Liu, Mike Travis, LKML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [-- Attachment #1: Type: text/plain, Size: 855 bytes --] On Thu, Apr 23, 2015 at 08:26:37AM -0500, Bjorn Helgaas wrote: > On Thu, Apr 23, 2015 at 09:55:13AM +0200, Ricardo Ribalda Delgado wrote: > > Hi Bjorn: > > > > On Wed, Apr 22, 2015 at 6:44 PM, Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote: > > > > > Usual style for referencing a commit is "(see 02bbde7849e6 ('Revert "of: > > > use platform_device_add"'))". > > > > Do you make that reference manually or there is a magic git command > > for printing it in that style? > > I used to do it mostly by hand, but thanks to your prompting, I fiddled > around and came up with this alias: > > gsr is aliased to `git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n"' There's also git log --format=fixes, though that includes the "Fixes: " prefix. That works with git show as well. Thierry [-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] of/platform: Use platform_device interface 2015-04-22 16:14 [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate Ricardo Ribalda Delgado ` (2 preceding siblings ...) 2015-04-22 16:14 ` [PATCH v3 3/4] base/platform: Continue on insert_resource() error Ricardo Ribalda Delgado @ 2015-04-22 16:14 ` Ricardo Ribalda Delgado [not found] ` <1429719261-18024-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 3 siblings, 1 reply; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-22 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel, devicetree Cc: Ricardo Ribalda Delgado of_platform_device_create_pdata() was using of_device_add() to create the devices, but of_platform_device_destroy was using of_platform_device_destroy(). of_device_add(), do not call insert_resource(), which initializes the parent field of the resource structure, needed by release_resource(), called by of_platform_device_destroy(). This leads to a NULL pointer deference. This patch, replaces of_device_add() with platform_device_data(). Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/of/platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index a01f57c..f011f57 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -183,8 +183,9 @@ static struct platform_device *of_platform_device_create_pdata( dev->dev.bus = &platform_bus_type; dev->dev.platform_data = platform_data; of_dma_configure(&dev->dev, dev->dev.of_node); + dev->name = dev_name(&dev->dev); - if (of_device_add(dev) != 0) { + if (platform_device_add(dev) != 0) { of_dma_deconfigure(&dev->dev); platform_device_put(dev); goto err_clear_flag; -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
[parent not found: <1429719261-18024-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 4/4] of/platform: Use platform_device interface [not found] ` <1429719261-18024-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-04-22 16:25 ` Rob Herring 2015-04-23 7:28 ` Ricardo Ribalda Delgado 2015-05-24 19:29 ` [PATCH v3.1 " Greg Kroah-Hartman 1 sibling, 1 reply; 16+ messages in thread From: Rob Herring @ 2015-04-22 16:25 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Wed, Apr 22, 2015 at 11:14 AM, Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > of_platform_device_create_pdata() was using of_device_add() to create > the devices, but of_platform_device_destroy was using > of_platform_device_destroy(). > > of_device_add(), do not call insert_resource(), which initializes the > parent field of the resource structure, needed by release_resource(), > called by of_platform_device_destroy(). > > This leads to a NULL pointer deference. > > This patch, replaces of_device_add() with platform_device_data(). This doesn't match the change. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > drivers/of/platform.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index a01f57c..f011f57 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -183,8 +183,9 @@ static struct platform_device *of_platform_device_create_pdata( > dev->dev.bus = &platform_bus_type; > dev->dev.platform_data = platform_data; > of_dma_configure(&dev->dev, dev->dev.of_node); > + dev->name = dev_name(&dev->dev); > > - if (of_device_add(dev) != 0) { > + if (platform_device_add(dev) != 0) { Can't we now remove of_device_add? > of_dma_deconfigure(&dev->dev); > platform_device_put(dev); > goto err_clear_flag; > -- > 2.1.4 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] of/platform: Use platform_device interface 2015-04-22 16:25 ` Rob Herring @ 2015-04-23 7:28 ` Ricardo Ribalda Delgado [not found] ` <CAPybu_3Ej2ecFDTiJVL9TfE=ZLFN2wLKE_nFL2WjMQtk+4LO_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-23 7:28 UTC (permalink / raw) To: Rob Herring Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Hello Rob On Wed, Apr 22, 2015 at 6:25 PM, Rob Herring <robherring2@gmail.com> wrote: >> This patch, replaces of_device_add() with platform_device_data(). > > This doesn't match the change. Thanks for catching it up. Will fix it and resend >> - if (of_device_add(dev) != 0) { >> + if (platform_device_add(dev) != 0) { > > Can't we now remove of_device_add? Now it is only used by ibmebus ricardo@neopili:~/linux-upstream$ git grep of_device_add arch/powerpc/kernel/ibmebus.c: ret = of_device_add(dev); drivers/of/device.c:int of_device_add(struct platform_device *ofdev) drivers/of/device.c: return of_device_add(pdev); include/linux/of_device.h:extern int of_device_add(struct platform_device *pdev); I think it can also use platform_device_add. I will prepare a patch to finally remove of_device_add() Thanks! -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAPybu_3Ej2ecFDTiJVL9TfE=ZLFN2wLKE_nFL2WjMQtk+4LO_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v3 4/4] of/platform: Use platform_device interface [not found] ` <CAPybu_3Ej2ecFDTiJVL9TfE=ZLFN2wLKE_nFL2WjMQtk+4LO_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-04-23 13:49 ` Ricardo Ribalda Delgado 0 siblings, 0 replies; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-04-23 13:49 UTC (permalink / raw) To: Rob Herring Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Hello Rob On Thu, Apr 23, 2015 at 9:28 AM, Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > I think it can also use platform_device_add. I will prepare a patch to > finally remove of_device_add() I will post right away an updated version of this patchset, and then I will prepare another one to remove of_device_add() completely. I already have two patches, but I have to figure out a proper way to test it. Thanks! -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3.1 4/4] of/platform: Use platform_device interface [not found] ` <1429719261-18024-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2015-04-22 16:25 ` Rob Herring @ 2015-05-24 19:29 ` Greg Kroah-Hartman [not found] ` <20150524192915.GA7170-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 1 sibling, 1 reply; 16+ messages in thread From: Greg Kroah-Hartman @ 2015-05-24 19:29 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA On Fri, May 15, 2015 at 01:52:10PM +0200, Ricardo Ribalda Delgado wrote: > of_platform_device_create_pdata() was using of_device_add() to create > the devices, but of_platform_device_destroy was using > platform_device_unregister() to free them. > > of_device_add(), do not call insert_resource(), which initializes the > parent field of the resource structure, needed by release_resource(), > called by of_platform_device_destroy(). This leads to a NULL pointer > deference. > > Instead of fixing the NULL pointer deference, what could hide other bugs, > this patch, replaces of_device_add() with platform_device_data(). > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > --- > > v3.1 Fix comments by Rob Herring, thanks! 3.1? Please resend the whole series, this is a mess, I can't find where this goes at all... greg k-h -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20150524192915.GA7170-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3.1 4/4] of/platform: Use platform_device interface [not found] ` <20150524192915.GA7170-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> @ 2015-05-26 7:26 ` Ricardo Ribalda Delgado 0 siblings, 0 replies; 16+ messages in thread From: Ricardo Ribalda Delgado @ 2015-05-26 7:26 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Grant Likely, Rob Herring, Andrew Morton, Jakub Sitnicki, Vivek Goyal, Bjorn Helgaas, Jiang Liu, Mike Travis, Thierry Reding, LKML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Hello Greg Sorry for the mess. I did not want to spam the mailing list too much. Repacking and resending. Thanks! On Sun, May 24, 2015 at 9:29 PM, Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote: > On Fri, May 15, 2015 at 01:52:10PM +0200, Ricardo Ribalda Delgado wrote: >> of_platform_device_create_pdata() was using of_device_add() to create >> the devices, but of_platform_device_destroy was using >> platform_device_unregister() to free them. >> >> of_device_add(), do not call insert_resource(), which initializes the >> parent field of the resource structure, needed by release_resource(), >> called by of_platform_device_destroy(). This leads to a NULL pointer >> deference. >> >> Instead of fixing the NULL pointer deference, what could hide other bugs, >> this patch, replaces of_device_add() with platform_device_data(). >> >> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> >> --- >> >> v3.1 Fix comments by Rob Herring, thanks! > > 3.1? > > Please resend the whole series, this is a mess, I can't find where this > goes at all... > > greg k-h -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-05-26 7:26 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-22 16:14 [PATCH v3 0/4] Fix null pointer deference when calling of_platform_depopulate Ricardo Ribalda Delgado
2015-04-22 16:14 ` [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Ricardo Ribalda Delgado
2015-04-22 16:47 ` Bjorn Helgaas
[not found] ` <20150422164701.GJ20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-04-23 8:06 ` Ricardo Ribalda Delgado
2015-04-22 16:14 ` [PATCH v3 2/4] base/platform: Only insert MEM and IO resources Ricardo Ribalda Delgado
2015-04-22 16:14 ` [PATCH v3 3/4] base/platform: Continue on insert_resource() error Ricardo Ribalda Delgado
[not found] ` <1429719261-18024-4-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-22 16:44 ` Bjorn Helgaas
[not found] ` <20150422164457.GI20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-04-23 7:55 ` Ricardo Ribalda Delgado
2015-04-23 13:26 ` Bjorn Helgaas
[not found] ` <20150423132637.GK20701-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-04-23 16:54 ` Thierry Reding
2015-04-22 16:14 ` [PATCH v3 4/4] of/platform: Use platform_device interface Ricardo Ribalda Delgado
[not found] ` <1429719261-18024-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-22 16:25 ` Rob Herring
2015-04-23 7:28 ` Ricardo Ribalda Delgado
[not found] ` <CAPybu_3Ej2ecFDTiJVL9TfE=ZLFN2wLKE_nFL2WjMQtk+4LO_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-23 13:49 ` Ricardo Ribalda Delgado
2015-05-24 19:29 ` [PATCH v3.1 " Greg Kroah-Hartman
[not found] ` <20150524192915.GA7170-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2015-05-26 7:26 ` Ricardo Ribalda Delgado
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).