[PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Armin Wolf]

The device name inside the ACPI netlink event is limited to
15 characters, so the WMI device name will get truncated.

This can be observed with kacpimon when receiving an event
from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":

	netlink:  9DBB5994-A997- 000000d0 00000000

Fix this by using the shorter device name from the ACPI
bus device instead. This still allows users to uniquely
identify the WMI device by using the notify id (0xd0).

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
Changes since v1:
- use acpi_dev_name() helper function
---
 drivers/platform/x86/wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index a7cfcbf92432..c61860db66ed 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1202,7 +1202,7 @@ static int wmi_notify_device(struct device *dev, void *data)
 	}

 	acpi_bus_generate_netlink_event(wblock->acpi_device->pnp.device_class,
-					dev_name(&wblock->dev.dev), *event, 0);
+					acpi_dev_name(wblock->acpi_device), *event, 0);

 	return -EBUSY;
 }
--
2.39.2

Re: [PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Hans de Goede]

Hi Armin,

On 1/21/24 21:08, Armin Wolf wrote:
> The device name inside the ACPI netlink event is limited to
> 15 characters, so the WMI device name will get truncated.
> 
> This can be observed with kacpimon when receiving an event
> from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":
> 
> 	netlink:  9DBB5994-A997- 000000d0 00000000
> 
> Fix this by using the shorter device name from the ACPI
> bus device instead. This still allows users to uniquely
> identify the WMI device by using the notify id (0xd0).
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
> Changes since v1:
> - use acpi_dev_name() helper function

I'm a bit divided on this patch. I agree the new way of doing
things is better, but technically this is a bit of a userspace API
break.

I guess we could hope that nothing depends on the old netlink API
format / name but I'm not sure we can rely on that ...

Ilpo, Andy do you have any opinion on this ?

Regards,

Hans




> ---
>  drivers/platform/x86/wmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index a7cfcbf92432..c61860db66ed 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -1202,7 +1202,7 @@ static int wmi_notify_device(struct device *dev, void *data)
>  	}
> 
>  	acpi_bus_generate_netlink_event(wblock->acpi_device->pnp.device_class,
> -					dev_name(&wblock->dev.dev), *event, 0);
> +					acpi_dev_name(wblock->acpi_device), *event, 0);
> 
>  	return -EBUSY;
>  }
> --
> 2.39.2
> 

Re: [PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Ilpo Järvinen]

On Mon, 22 Jan 2024, Hans de Goede wrote:
> On 1/21/24 21:08, Armin Wolf wrote:
> > The device name inside the ACPI netlink event is limited to
> > 15 characters, so the WMI device name will get truncated.
> > 
> > This can be observed with kacpimon when receiving an event
> > from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":
> > 
> > 	netlink:  9DBB5994-A997- 000000d0 00000000
> > 
> > Fix this by using the shorter device name from the ACPI
> > bus device instead. This still allows users to uniquely
> > identify the WMI device by using the notify id (0xd0).
> > 
> > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > ---
> > Changes since v1:
> > - use acpi_dev_name() helper function
> 
> I'm a bit divided on this patch. I agree the new way of doing
> things is better, but technically this is a bit of a userspace API
> break.
> 
> I guess we could hope that nothing depends on the old netlink API
> format / name but I'm not sure we can rely on that ...
> 
> Ilpo, Andy do you have any opinion on this ?

Hi Armin,

Have you tried to conduct debian code search to find the code using this?

-- 
 i.

> > ---
> >  drivers/platform/x86/wmi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> > index a7cfcbf92432..c61860db66ed 100644
> > --- a/drivers/platform/x86/wmi.c
> > +++ b/drivers/platform/x86/wmi.c
> > @@ -1202,7 +1202,7 @@ static int wmi_notify_device(struct device *dev, void *data)
> >  	}
> > 
> >  	acpi_bus_generate_netlink_event(wblock->acpi_device->pnp.device_class,
> > -					dev_name(&wblock->dev.dev), *event, 0);
> > +					acpi_dev_name(wblock->acpi_device), *event, 0);
> > 
> >  	return -EBUSY;
> >  }
> > --
> > 2.39.2
> > 
> 

Re: [PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Armin Wolf]

Am 22.01.24 um 13:47 schrieb Ilpo Järvinen:

> On Mon, 22 Jan 2024, Hans de Goede wrote:
>> On 1/21/24 21:08, Armin Wolf wrote:
>>> The device name inside the ACPI netlink event is limited to
>>> 15 characters, so the WMI device name will get truncated.
>>>
>>> This can be observed with kacpimon when receiving an event
>>> from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":
>>>
>>> 	netlink:  9DBB5994-A997- 000000d0 00000000
>>>
>>> Fix this by using the shorter device name from the ACPI
>>> bus device instead. This still allows users to uniquely
>>> identify the WMI device by using the notify id (0xd0).
>>>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> ---
>>> Changes since v1:
>>> - use acpi_dev_name() helper function
>> I'm a bit divided on this patch. I agree the new way of doing
>> things is better, but technically this is a bit of a userspace API
>> break.
>>
>> I guess we could hope that nothing depends on the old netlink API
>> format / name but I'm not sure we can rely on that ...
>>
>> Ilpo, Andy do you have any opinion on this ?
> Hi Armin,
>
> Have you tried to conduct debian code search to find the code using this?
>
I did, but i found nothing.

I am also preparing to properly initialize the device_class field in another patch,
and this also has the potential of breaking userspace if some users created custom
acpid configs to listen to those events.

I think we can revert those changes anytime if someones turns out to be actually depending
on them.

Armin Wolf

Re: [PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Hans de Goede]

Hi,

On 1/22/24 16:03, Armin Wolf wrote:
> Am 22.01.24 um 13:47 schrieb Ilpo Järvinen:
> 
>> On Mon, 22 Jan 2024, Hans de Goede wrote:
>>> On 1/21/24 21:08, Armin Wolf wrote:
>>>> The device name inside the ACPI netlink event is limited to
>>>> 15 characters, so the WMI device name will get truncated.
>>>>
>>>> This can be observed with kacpimon when receiving an event
>>>> from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":
>>>>
>>>>     netlink:  9DBB5994-A997- 000000d0 00000000
>>>>
>>>> Fix this by using the shorter device name from the ACPI
>>>> bus device instead. This still allows users to uniquely
>>>> identify the WMI device by using the notify id (0xd0).
>>>>
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> ---
>>>> Changes since v1:
>>>> - use acpi_dev_name() helper function
>>> I'm a bit divided on this patch. I agree the new way of doing
>>> things is better, but technically this is a bit of a userspace API
>>> break.
>>>
>>> I guess we could hope that nothing depends on the old netlink API
>>> format / name but I'm not sure we can rely on that ...
>>>
>>> Ilpo, Andy do you have any opinion on this ?
>> Hi Armin,
>>
>> Have you tried to conduct debian code search to find the code using this?
>>
> I did, but i found nothing.
> 
> I am also preparing to properly initialize the device_class field in another patch,
> and this also has the potential of breaking userspace if some users created custom
> acpid configs to listen to those events.
> 
> I think we can revert those changes anytime if someones turns out to be actually depending
> on them.

Based on the above I'm willing to go ahead with these changes, with
the note that we need to be prepared to revert this immediately if we
get any reports if this breaking userspace.

Given the (unlikely) potential breakage I think it would be good
to get this change into linux-next soon for maximum testing.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

Re: [PATCH v2] platform/x86: wmi: Use ACPI device name in netlink event

[Ilpo Järvinen]

On Sun, 21 Jan 2024 21:08:24 +0100, Armin Wolf wrote:

> The device name inside the ACPI netlink event is limited to
> 15 characters, so the WMI device name will get truncated.
> 
> This can be observed with kacpimon when receiving an event
> from WMI device "9DBB5994-A997-11DA-B012-B622A1EF5492":
> 
> 	netlink:  9DBB5994-A997- 000000d0 00000000
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86: wmi: Use ACPI device name in netlink event
      commit: 3f399b5d7189bcb608c75abc85fe39f7a5509cfa

--
 i.