diff mbox

[1/1] sdhci-pci.c: a fix for invalid memory access

Message ID 4f05d51210800874cc706a6f657e297491891b41.1396297182.git.nitin.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nitin A Kamble March 31, 2014, 8:51 p.m. UTC 
From: Nitin A Kamble <nitin.a.kamble@intel.com>

The chip->fixes pointer is assumed to be a valid data pointer, which is
not always the case. Fox eaxample, this code was giving a kernel panic for
emenlow BSP, because the chip->fixes pointer is NULL.

The bug was introduced in the LTSI code by this patch:
patches.baytrail/1193-PENDING-mmc-sdhci-pci-Fix-BYT-sd-card-getting-stuck-.patch
 From 8ba911fd25c8f41979533f6911f8221181fcefae Mon Sep 17 00:00:00 2001
 From: Adrian Hunter <adrian.hunter@intel.com>
 Date: Fri, 13 Dec 2013 13:57:34 -0800
 Subject: PENDING: mmc: sdhci-pci: Fix BYT sd card getting stuck in runtime suspend


Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com>

---
 drivers/mmc/host/sdhci-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nitin A Kamble March 31, 2014, 9:17 p.m. UTC | #1 
On 3/31/2014 1:51 PM, nitin.a.kamble@intel.com wrote:
> From: Nitin A Kamble <nitin.a.kamble@intel.com>
>
> The chip->fixes pointer is assumed to be a valid data pointer, which is
> not always the case. Fox eaxample, this code was giving a kernel panic for
> emenlow BSP, because the chip->fixes pointer is NULL.
>
> The bug was introduced in the LTSI code by this patch:
> patches.baytrail/1193-PENDING-mmc-sdhci-pci-Fix-BYT-sd-card-getting-stuck-.patch
>   From 8ba911fd25c8f41979533f6911f8221181fcefae Mon Sep 17 00:00:00 2001
>   From: Adrian Hunter <adrian.hunter@intel.com>
>   Date: Fri, 13 Dec 2013 13:57:34 -0800
>   Subject: PENDING: mmc: sdhci-pci: Fix BYT sd card getting stuck in runtime suspend
>
>
> Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com>
>
> ---
>   drivers/mmc/host/sdhci-pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index 33593e7..b7c8e20 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -1389,7 +1389,7 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>   	 * from runtime suspend.  If it is not there, don't allow runtime PM.
>   	 * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on failure.
>   	 */
> -	if (chip->fixes->own_cd_for_runtime_pm && !gpio_is_valid(slot->cd_gpio))
> +	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm && !gpio_is_valid(slot->cd_gpio))
>   		chip->allow_runtime_pm = false;
>   
>   	return slot;
FYI,
I just found out that similar fix is already in the linux-stable git 
repository.

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/mmc/host/sdhci-pci.c?id=945be38caa287b177b8c17ffaae7754cab6a658f

So my fix is not needed anymore. Instead the linux-stable commit 
(0x945be38c) can be pulled into LTSI repository to fix the kernel panic 
on boards like emenlow.

Thanks,
Nitin
Bruce Ashfield March 31, 2014, 9:43 p.m. UTC | #2 
On 2014-03-31, 5:17 PM, Kamble, Nitin A wrote:
>
> On 3/31/2014 1:51 PM, nitin.a.kamble@intel.com wrote:
>> From: Nitin A Kamble <nitin.a.kamble@intel.com>
>>
>> The chip->fixes pointer is assumed to be a valid data pointer, which is
>> not always the case. Fox eaxample, this code was giving a kernel panic
>> for
>> emenlow BSP, because the chip->fixes pointer is NULL.
>>
>> The bug was introduced in the LTSI code by this patch:
>> patches.baytrail/1193-PENDING-mmc-sdhci-pci-Fix-BYT-sd-card-getting-stuck-.patch
>>
>>   From 8ba911fd25c8f41979533f6911f8221181fcefae Mon Sep 17 00:00:00 2001
>>   From: Adrian Hunter <adrian.hunter@intel.com>
>>   Date: Fri, 13 Dec 2013 13:57:34 -0800
>>   Subject: PENDING: mmc: sdhci-pci: Fix BYT sd card getting stuck in
>> runtime suspend
>>
>>
>> Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com>
>>
>> ---
>>   drivers/mmc/host/sdhci-pci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
>> index 33593e7..b7c8e20 100644
>> --- a/drivers/mmc/host/sdhci-pci.c
>> +++ b/drivers/mmc/host/sdhci-pci.c
>> @@ -1389,7 +1389,7 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>>        * from runtime suspend.  If it is not there, don't allow
>> runtime PM.
>>        * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on
>> failure.
>>        */
>> -    if (chip->fixes->own_cd_for_runtime_pm &&
>> !gpio_is_valid(slot->cd_gpio))
>> +    if (chip->fixes && chip->fixes->own_cd_for_runtime_pm &&
>> !gpio_is_valid(slot->cd_gpio))
>>           chip->allow_runtime_pm = false;
>>       return slot;
> FYI,
> I just found out that similar fix is already in the linux-stable git
> repository.
>
> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/mmc/host/sdhci-pci.c?id=945be38caa287b177b8c17ffaae7754cab6a658f
>
>
> So my fix is not needed anymore. Instead the linux-stable commit
> (0x945be38c) can be pulled into LTSI repository to fix the kernel panic
> on boards like emenlow.

Unless I'm mistaken, that's actually a mainline commit that is in 3.14.

 > git describe 945be38caa287b177b8c17ffaae7754cab6a658f
v3.13-rc1-183-g945be38caa28

So you see it in linux-stable by virtue of greg pulling in Linus' tree.

It still needs to be sent to -stable, LTSI and I can cherry pick it as
well, but it isn't already in the stable merges.

Bruce

>
> Thanks,
> Nitin
Bruce Ashfield March 31, 2014, 10:14 p.m. UTC | #3 
On 2014-03-31, 6:04 PM, Greg KH wrote:
> On Mon, Mar 31, 2014 at 05:43:33PM -0400, Bruce Ashfield wrote:
>> On 2014-03-31, 5:17 PM, Kamble, Nitin A wrote:
>>>
>>> On 3/31/2014 1:51 PM, nitin.a.kamble@intel.com wrote:
>>>> From: Nitin A Kamble <nitin.a.kamble@intel.com>
>>>>
>>>> The chip->fixes pointer is assumed to be a valid data pointer, which is
>>>> not always the case. Fox eaxample, this code was giving a kernel panic
>>>> for
>>>> emenlow BSP, because the chip->fixes pointer is NULL.
>>>>
>>>> The bug was introduced in the LTSI code by this patch:
>>>> patches.baytrail/1193-PENDING-mmc-sdhci-pci-Fix-BYT-sd-card-getting-stuck-.patch
>>>>
>>>>   From 8ba911fd25c8f41979533f6911f8221181fcefae Mon Sep 17 00:00:00 2001
>>>>   From: Adrian Hunter <adrian.hunter@intel.com>
>>>>   Date: Fri, 13 Dec 2013 13:57:34 -0800
>>>>   Subject: PENDING: mmc: sdhci-pci: Fix BYT sd card getting stuck in
>>>> runtime suspend
>>>>
>>>>
>>>> Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com>
>>>>
>>>> ---
>>>>   drivers/mmc/host/sdhci-pci.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
>>>> index 33593e7..b7c8e20 100644
>>>> --- a/drivers/mmc/host/sdhci-pci.c
>>>> +++ b/drivers/mmc/host/sdhci-pci.c
>>>> @@ -1389,7 +1389,7 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>>>>        * from runtime suspend.  If it is not there, don't allow
>>>> runtime PM.
>>>>        * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on
>>>> failure.
>>>>        */
>>>> -    if (chip->fixes->own_cd_for_runtime_pm &&
>>>> !gpio_is_valid(slot->cd_gpio))
>>>> +    if (chip->fixes && chip->fixes->own_cd_for_runtime_pm &&
>>>> !gpio_is_valid(slot->cd_gpio))
>>>>           chip->allow_runtime_pm = false;
>>>>       return slot;
>>> FYI,
>>> I just found out that similar fix is already in the linux-stable git
>>> repository.
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/mmc/host/sdhci-pci.c?id=945be38caa287b177b8c17ffaae7754cab6a658f
>>>
>>>
>>> So my fix is not needed anymore. Instead the linux-stable commit
>>> (0x945be38c) can be pulled into LTSI repository to fix the kernel panic
>>> on boards like emenlow.
>>
>> Unless I'm mistaken, that's actually a mainline commit that is in 3.14.
>>
>>> git describe 945be38caa287b177b8c17ffaae7754cab6a658f
>> v3.13-rc1-183-g945be38caa28
>
> Yes, it showed up in 3.14-rc1 (use --contains for git describe)

:) Thanks. My git tag --contains shows the same thing on my latest tree.

>
>> So you see it in linux-stable by virtue of greg pulling in Linus' tree.
>>
>> It still needs to be sent to -stable, LTSI and I can cherry pick it as
>> well, but it isn't already in the stable merges.
>
> It already showed up in 3.13.3, but it never made it to 3.10-stable as
> it did not apply there.

Ack'd.

Nitin: which means my point stands, if you want this sooner than it
showing up in the yocto tree via the 3.10 LTSI, you'll want me to
cherry pick it (or port it to 3.10 like it was for LTSI and get it into 
-stable, but Greg will advise you if that's ok).

Let me know which route you want to take .. it's all roughly the same
to me.

Bruce

>
> greg k-h
>
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index 33593e7..b7c8e20 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1389,7 +1389,7 @@  static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	 * from runtime suspend.  If it is not there, don't allow runtime PM.
 	 * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on failure.
 	 */
-	if (chip->fixes->own_cd_for_runtime_pm && !gpio_is_valid(slot->cd_gpio))
+	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm && !gpio_is_valid(slot->cd_gpio))
 		chip->allow_runtime_pm = false;
 
 	return slot;