On 05/18/2017 01:35 PM, Verma, Vishal L wrote:
> On Wed, 2017-05-17 at 19:56 -0400, Prarit Bhargava wrote:
>> nfit_init() calls nfit_mce_register() on module load. When the module
>> load fails the nfit mce decoder is not unregistered. The module's
>> memory is freed leaving the decoder chain referencing junk. This will
>> cause panics as future registrations will reference the free'd memory.
>>
>> Only register the nfit mce decoder on success.
>
> Good find! I'm wondering if registering the mce handler after the rest
> of the init will leave a (small) window open where MCEs can happen but
> the handler has yet to be registered.. Maybe it is inconsequential, and
> we can just do this. Alternatively, if acpi_bus_register_driver fails,
> then go back and unregister the mce handler..
FWIW my first version did exactly what you describe. I felt like the version
below was a cleaner approach in terms of code, and because I thought that the
possibility of an MCE occurring in those few moments between the return and
registration was minimal.
I could be wrong though :) and I still have that original patch so it isn't a
big deal to do a v2.
P.
>
>>
>> Signed-off-by: Prarit Bhargava <prarit@xxxxxxxxxx>
>> Cc: Jeff Moyer <jmoyer@xxxxxxxxxx>
>> Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
>> Cc: Len Brown <lenb@xxxxxxxxxx>
>> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
>> Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
>> Cc: "Lee, Chun-Yi" <joeyli.kernel@xxxxxxxxx>
>> Cc: Linda Knippers <linda.knippers@xxxxxxx>
>> ---
>> drivers/acpi/nfit/core.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 656acb5d7166..b60856b4325b 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -3043,6 +3043,8 @@ static void acpi_nfit_notify(struct acpi_device
>> *adev, u32 event)
>>
>> static __init int nfit_init(void)
>> {
>> + int ret;
>> +
>> BUILD_BUG_ON(sizeof(struct acpi_table_nfit) != 40);
>> BUILD_BUG_ON(sizeof(struct acpi_nfit_system_address) != 56);
>> BUILD_BUG_ON(sizeof(struct acpi_nfit_memory_map) != 48);
>> @@ -3069,9 +3071,11 @@ static __init int nfit_init(void)
>> if (!nfit_wq)
>> return -ENOMEM;
>>
>> - nfit_mce_register();
>> + ret = acpi_bus_register_driver(&acpi_nfit_driver);
>> + if (!ret)
>> + nfit_mce_register();
>>
>> - return acpi_bus_register_driver(&acpi_nfit_driver);
>> + return ret;
>> }
>>
>> static __exit void nfit_exit(void)
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
