Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1280754 > unrolled thread

Re: [PATCH] base/platform: fix panic when probe function is NULL

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] base/platform: fix panic when probe function is NULL "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-01 11:50 +0100
    Re: [PATCH] base/platform: fix panic when probe function is NULL Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-01 14:30 +0100
      Re: [PATCH] base/platform: fix panic when probe function is NULL "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-01 16:20 +0100
        Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 18:30 +0100
          Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL Peter Huewe <peterhuewe@gmx.de> - 2015-12-01 19:40 +0100
            Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 19:50 +0100
              Aw: Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL "Peter Huewe" <PeterHuewe@gmx.de> - 2015-12-01 20:00 +0100
                Re: Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:10 +0100

#1280754 — Re: [PATCH] base/platform: fix panic when probe function is NULL

SGVsbG8gVXdlLAoKPiBUaGlzIHNvdW5kcyBsaWtlIGEgc2VwYXJhdGUgaXNzdWUgdGhvdWdoLiBM
b29raW5nIGF0IGluaXRfdGlzIHRoZXJlIGlzOgo+IAo+ICAgICAgICAgcmMgPSBwbGF0Zm9ybV9k
cml2ZXJfcmVnaXN0ZXIoJnRpc19kcnYpOwo+ICAgICAgICAgaWYgKHJjIDwgMCkKPiAgICAgICAg
ICAgICAgICAgcmV0dXJuIHJjOwo+ICAgICAgICAgcGRldiA9IHBsYXRmb3JtX2RldmljZV9yZWdp
c3Rlcl9zaW1wbGUoInRwbV90aXMiLCAtMSwgTlVMTCwgMCk7Cj4gICAgICAgICBpZiAoSVNfRVJS
KHBkZXYpKSB7Cj4gICAgICAgICAgICAgICAgIHJjID0gUFRSX0VSUihwZGV2KTsKPiAgICAgICAg
ICAgICAgICAgZ290byBlcnJfZGV2Owo+ICAgICAgICAgfQo+ICAgICAgICAgcmMgPSB0cG1fdGlz
X2luaXQoJnBkZXYtPmRldiwgJnRpc19kZWZhdWx0X2luZm8sIE5VTEwpOwo+IAo+IHRwbV90aXNf
aW5pdCBjYWxscyB0cG1tX2NoaXBfYWxsb2Mgd2hpY2ggYmFyZnMgd2hlbiBwZGV2IChpLmUuIHRo
ZSByZXR1cm4gdmFsdWUKPiBvZiBwbGF0Zm9ybV9kZXZpY2VfcmVnaXN0ZXJfc2ltcGxlIGFib3Zl
KSBpc24ndCBib3VuZC4gSXQgaXMgbm90IGFsbG93ZWQKPiB0byBhc3N1bWUgdGhhdCB0aGUgZGV2
aWNlIGlzIGJvdW5kIGFmdGVyIHRoZSBhYm92ZSBmdW5jdGlvbiBjYWxscy4KCkNhbiB5b3UgcGxl
YXNlIGV4cGxhaW4gYWdhaW4gd2h5IHlvdSB0aGluayB0aGF0IGFzc3VtcHRpb24gaXMgaW52YWxp
ZD8gCkFzIGZhciBhcyBJIHVuZGVyc3RhbmQgdGhlIGNvZGUsIHRoZSBhc3N1bXB0aW9uIHdvdWxk
IGJlIGNvcnJlY3QgaW4KNC4zLjAgYW5kIGVhcmxpZXI6CgpwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0
ZXIoKSByZWdpc3RlcnMgYSBwbGF0Zm9ybSBkcml2ZXIgd2l0aCBuYW1lCiJ0cG1fdGlzIi4gcGxh
dGZvcm1fZGV2aWNlX3JlZ2lzdGVyX3NpbXBsZSgpIHJlZ2lzdGVycyBhIGRldmljZSB3aXRoIHRo
ZQpzYW1lIG5hbWUuIFRoaXMgd2lsbCBjYWxsIHBsYXRmb3JtX2RldmljZV9hZGQoKS9kZXZpY2Vf
YWRkKCkgYW5kIHN0YXJ0CnByb2JpbmcgZm9yIGEgcGxhdGZvcm0gZGV2aWNlLiBQbGF0Zm9ybSBi
dXMgcHJvYmluZyBpbiBwbGF0Zm9ybV9tYXRjaCgpCmZhbGxzIGJhY2sgdG8gYSBzaW1wbGUgbWF0
Y2ggYmV0d2VlbiBkcml2ZXIgYW5kIGRldmljZSBuYW1lIGlmIGFsbCBlbHNlCmZhaWxzLiBUaGF0
IG1hdGNoIHN1Y2NlZWRzIGZvciB0aGUgInRwbV90aXMiIGRyaXZlci4gVGh1cwpkcml2ZXJfcHJv
YmVfZGV2aWNlKCkgd2lsbCBiZSBjYWxsZWQsIGFuZCBpbiB0aGUgYWJzZW5jZSBvZiBhCmRyaXZl
ci1zcGVjaWZpYyBwcm9iZSByb3V0aW5lLCB3aWxsIHN1Y2NlZWQuIFRodXMgYWZ0ZXIKcGxhdGZv
cm1fZGV2aWNlX3JlZ2lzdGVyX3NpbXBsZSgpIHJldHVybnMsIGRldmljZSBhbmQgZHJpdmVyIHdp
bGwgYmUKYm91bmQuIFRoaXMgbWF0Y2hlcyBhbHNvIGFjdHVhbCBiZWhhdmlvciBvZiB0aGUgcHJl
LTQuNCBjb2RlLiAKClBsZWFzZSBleHBsYWluIHdoYXQgSSBhbSBvdmVybG9va2luZy4gSSBhbSBq
dXN0IHRyeWluZyB0byB1bmRlcnN0YW5kLgpBcyBmYXIgYXMgdHBtX3RpcyBpcyBjb25jZXJuZWQs
IEphc29uJ3MgY3VycmVudCBwYXRjaCBzZXQgaXMgZ29pbmcgdG8KZml4IHRoaXMgZm9yIGdvb2Qg
YW55d2F5LgoKUmVnYXJkcwpNYXJ0aW4KCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1280863

Hello Martin,

On Tue, Dec 01, 2015 at 11:41:53AM +0100, Wilck, Martin wrote:
> > This sounds like a separate issue though. Looking at init_tis there is:
> > 
> >         rc = platform_driver_register(&tis_drv);
> >         if (rc < 0)
> >                 return rc;
> >         pdev = platform_device_register_simple("tpm_tis", -1, NULL, 0);
> >         if (IS_ERR(pdev)) {
> >                 rc = PTR_ERR(pdev);
> >                 goto err_dev;
> >         }
> >         rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL);
> > 
> > tpm_tis_init calls tpmm_chip_alloc which barfs when pdev (i.e. the return value
> > of platform_device_register_simple above) isn't bound. It is not allowed
> > to assume that the device is bound after the above function calls.
> 
> Can you please explain again why you think that assumption is invalid? 

You can unbind a device from a driver via sysfs, you can also prevent
binding somehow I think, probing can fail for different reasons, probing
might wait for userspace interaction to load firmware which wasn't
scheduled yet. I'm sure there are still more things that break the
assumption.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280939

PiA+ID4gdHBtX3Rpc19pbml0IGNhbGxzIHRwbW1fY2hpcF9hbGxvYyB3aGljaCBiYXJmcyB3aGVu
IHBkZXYgKGkuZS4gdGhlIHJldHVybiB2YWx1ZQo+ID4gPiBvZiBwbGF0Zm9ybV9kZXZpY2VfcmVn
aXN0ZXJfc2ltcGxlIGFib3ZlKSBpc24ndCBib3VuZC4gSXQgaXMgbm90IGFsbG93ZWQKPiA+ID4g
dG8gYXNzdW1lIHRoYXQgdGhlIGRldmljZSBpcyBib3VuZCBhZnRlciB0aGUgYWJvdmUgZnVuY3Rp
b24gY2FsbHMuCj4gPiAKPiA+IENhbiB5b3UgcGxlYXNlIGV4cGxhaW4gYWdhaW4gd2h5IHlvdSB0
aGluayB0aGF0IGFzc3VtcHRpb24gaXMgaW52YWxpZD8gCj4gCj4gWW91IGNhbiB1bmJpbmQgYSBk
ZXZpY2UgZnJvbSBhIGRyaXZlciB2aWEgc3lzZnMsIHlvdSBjYW4gYWxzbyBwcmV2ZW50Cj4gYmlu
ZGluZyBzb21laG93IEkgdGhpbmssIHByb2JpbmcgY2FuIGZhaWwgZm9yIGRpZmZlcmVudCByZWFz
b25zLCBwcm9iaW5nCj4gbWlnaHQgd2FpdCBmb3IgdXNlcnNwYWNlIGludGVyYWN0aW9uIHRvIGxv
YWQgZmlybXdhcmUgd2hpY2ggd2Fzbid0Cj4gc2NoZWR1bGVkIHlldC4gSSdtIHN1cmUgdGhlcmUg
YXJlIHN0aWxsIG1vcmUgdGhpbmdzIHRoYXQgYnJlYWsgdGhlCj4gYXNzdW1wdGlvbi4KClRoYW5r
cy4gT3V0IG9mIHRoZXNlLCAicHJldmVudCBiaW5kaW5nIHNvbWVob3ciIHdvdWxkIGJlIHRoZSBv
bmx5CnByb2JsZW0gdGhhdCBhcHBsaWVzIHRvIHRwbV90aXMsIGFzIHByb2JpbmcgY2FuJ3QgZmFp
bCAobm8gcHJvYmUoKQpyb3V0aW5lKSwgdGhlcmUncyBubyBGVyB0byBsb2FkLCBhbmQgdW5iaW5k
aW5nIHZpYSBzeXNmcyB3b3VsZCByZXF1aXJlCm5lYXJseSBpbXBvc3NpYmxlIHRpbWluZyAobm90
IHN1cmUgaWYgaXQgY291bGQgYmUgZG9uZSB3aXRoIHVkZXYpLgoKQW55d2F5LCB0aGUgUmlnaHQg
VGhpbmcgdG8gZG8gaXMgdG8gY3JlYXRlIGEgcHJvYmUoKSByb3V0aW5lIGFuZCB0aGF0J3MKd2hh
dCBKYXNvbiBkaWQuCgpUaGFua3MgYWdhaW4sCk1hcnRpbgoK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281071 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Tue, Dec 01, 2015 at 04:19:25PM +0100, Wilck, Martin wrote:
> > > > tpm_tis_init calls tpmm_chip_alloc which barfs when pdev (i.e. the return value
> > > > of platform_device_register_simple above) isn't bound. It is not allowed
> > > > to assume that the device is bound after the above function calls.
> > > 
> > > Can you please explain again why you think that assumption is invalid? 
> > 
> > You can unbind a device from a driver via sysfs, you can also prevent
> > binding somehow I think, probing can fail for different reasons, probing
> > might wait for userspace interaction to load firmware which wasn't
> > scheduled yet. I'm sure there are still more things that break the
> > assumption.
> 
> Thanks. Out of these, "prevent binding somehow" would be the only
> problem that applies to tpm_tis, as probing can't fail (no probe()
> routine), there's no FW to load, and unbinding via sysfs would require
> nearly impossible timing (not sure if it could be done with udev).
> 
> Anyway, the Right Thing to do is to create a probe() routine and that's
> what Jason did.

That fixes tpm_tis, but there are other ancient TPM drivers that use
the old, now broken way.

So, we still need to do something here. Either fixup b8b2c7d845d5 as
you have proposed, remove the now broken obsolete TPM drivers, or try
and fix them..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281108 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

Am 1. Dezember 2015 09:25:37 PST, schrieb Jason Gunthorpe <jgunthorpe@obsidianresearch.com>:
>On Tue, Dec 01, 2015 at 04:19:25PM +0100, Wilck, Martin wrote:
>> > > > tpm_tis_init calls tpmm_chip_alloc which barfs when pdev (i.e.
>the return value
>> > > > of platform_device_register_simple above) isn't bound. It is
>not allowed
>> > > > to assume that the device is bound after the above function
>calls.
>> > > 
>> > > Can you please explain again why you think that assumption is
>invalid? 
>> > 
>> > You can unbind a device from a driver via sysfs, you can also
>prevent
>> > binding somehow I think, probing can fail for different reasons,
>probing
>> > might wait for userspace interaction to load firmware which wasn't
>> > scheduled yet. I'm sure there are still more things that break the
>> > assumption.
>> 
>> Thanks. Out of these, "prevent binding somehow" would be the only
>> problem that applies to tpm_tis, as probing can't fail (no probe()
>> routine), there's no FW to load, and unbinding via sysfs would
>require
>> nearly impossible timing (not sure if it could be done with udev).
>> 
>> Anyway, the Right Thing to do is to create a probe() routine and
>that's
>> what Jason did.
>
>That fixes tpm_tis, but there are other ancient TPM drivers that use
>the old, now broken way.
>
>So, we still need to do something here. Either fixup b8b2c7d845d5 as
>you have proposed, remove the now broken obsolete TPM drivers, or try
>and fix them..

How broken are they and since when?

I thought multiple times about deprecating and finally removing the 1.1b stuff - tpm 1.2 is out for 10? years now? With an expected life span of a TPM of roughly 5years... 
And also unfortunately the 1.1b legacy drivers usually get loaded first :( (atleast for slb9635)

Mark them as obsolete , default them to No and remove them by 4.10 if there are no objections?

Peter
-- 
Sent from my mobile
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281111 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Tue, Dec 01, 2015 at 10:26:17AM -0800, Peter Huewe wrote:
> 
> 
> Am 1. Dezember 2015 09:25:37 PST, schrieb Jason Gunthorpe <jgunthorpe@obsidianresearch.com>:
> >On Tue, Dec 01, 2015 at 04:19:25PM +0100, Wilck, Martin wrote:
> >> > > > tpm_tis_init calls tpmm_chip_alloc which barfs when pdev (i.e.
> >the return value
> >> > > > of platform_device_register_simple above) isn't bound. It is
> >not allowed
> >> > > > to assume that the device is bound after the above function
> >calls.
> >> > > 
> >> > > Can you please explain again why you think that assumption is
> >invalid? 
> >> > 
> >> > You can unbind a device from a driver via sysfs, you can also
> >prevent
> >> > binding somehow I think, probing can fail for different reasons,
> >probing
> >> > might wait for userspace interaction to load firmware which wasn't
> >> > scheduled yet. I'm sure there are still more things that break the
> >> > assumption.
> >> 
> >> Thanks. Out of these, "prevent binding somehow" would be the only
> >> problem that applies to tpm_tis, as probing can't fail (no probe()
> >> routine), there's no FW to load, and unbinding via sysfs would
> >require
> >> nearly impossible timing (not sure if it could be done with udev).
> >> 
> >> Anyway, the Right Thing to do is to create a probe() routine and
> >that's
> >> what Jason did.
> >
> >That fixes tpm_tis, but there are other ancient TPM drivers that use
> >the old, now broken way.
> >
> >So, we still need to do something here. Either fixup b8b2c7d845d5 as
> >you have proposed, remove the now broken obsolete TPM drivers, or try
> >and fix them..
> 
> How broken are they and since when?

oops the kernel broken, since 4.4-rc1 apparently, so not released yet.

> Mark them as obsolete , default them to No and remove them by 4.10
> if there are no objections?

Greg KH has been advising just to delete stuff right away. It is easy
to undelete something if someone comes around with hardware and is
willing to test

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281122 — Aw: Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

> > >That fixes tpm_tis, but there are other ancient TPM drivers that use
> > >the old, now broken way.
> > >
> > >So, we still need to do something here. Either fixup b8b2c7d845d5 as
> > >you have proposed, remove the now broken obsolete TPM drivers, or try
> > >and fix them..
> >
> > How broken are they and since when?

> oops the kernel broken, since 4.4-rc1 apparently, so not released yet.
damn, I was hoping MUCH longer :)

> > Mark them as obsolete , default them to No and remove them by 4.10
> > if there are no objections?

> Greg KH has been advising just to delete stuff right away. It is easy
> to undelete something if someone comes around with hardware and is
> willing to test

Can you point to that discussion or was it offline?
I mean for staging, yes there it is clear, but for stuff that is officially "upstream", I'm not 100% sure.

Thanks,
Peter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281131 — Re: Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Tue, Dec 01, 2015 at 07:54:59PM +0100, Peter Huewe wrote:
> Can you point to that discussion or was it offline?

Apparently it was brought up at the most recent kernel summit

http://www.spinics.net/lists/linux-rdma/msg29985.html

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web