Message ID | 20240219115919.16526-3-W_Armin@gmx.de (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
Series | platform/x86: wmi: Fixes for event data handling | expand |
Am 19.02.24 um 12:59 schrieb Armin Wolf: > WMI event drivers which do not have no_notify_data set expect > that each WMI event contains valid data. Evaluating _WED however > might return no data, which can cause issues with such drivers. > > Fix this by validating that evaluating _WED did return data. > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > --- > drivers/platform/x86/wmi.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 8fb90b726f50..d0fe8153f803 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -1210,6 +1210,7 @@ static void wmi_notify_driver(struct wmi_block *wblock) > { > struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); > struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *obj = NULL; > acpi_status status; > > if (!driver->no_notify_data) { > @@ -1218,12 +1219,18 @@ static void wmi_notify_driver(struct wmi_block *wblock) > dev_warn(&wblock->dev.dev, "Failed to get event data\n"); > return; > } > + > + obj = data.pointer; > + if (!obj) { > + dev_warn(&wblock->dev.dev, "Event contains not event data\n"); I just noticed that this should have been "Event contains no event data\n". Should i send another patch? Armin Wolf > + return; > + } > } > > if (driver->notify) > - driver->notify(&wblock->dev, data.pointer); > + driver->notify(&wblock->dev, obj); > > - kfree(data.pointer); > + kfree(obj); > } > > static int wmi_notify_device(struct device *dev, void *data) > -- > 2.39.2 > >
On Tue, 20 Feb 2024, Armin Wolf wrote: > Am 19.02.24 um 12:59 schrieb Armin Wolf: > > > WMI event drivers which do not have no_notify_data set expect > > that each WMI event contains valid data. Evaluating _WED however > > might return no data, which can cause issues with such drivers. > > > > Fix this by validating that evaluating _WED did return data. > > > > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > > --- > > drivers/platform/x86/wmi.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > > index 8fb90b726f50..d0fe8153f803 100644 > > --- a/drivers/platform/x86/wmi.c > > +++ b/drivers/platform/x86/wmi.c > > @@ -1210,6 +1210,7 @@ static void wmi_notify_driver(struct wmi_block > > *wblock) > > { > > struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); > > struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; > > + union acpi_object *obj = NULL; > > acpi_status status; > > > > if (!driver->no_notify_data) { > > @@ -1218,12 +1219,18 @@ static void wmi_notify_driver(struct wmi_block > > *wblock) > > dev_warn(&wblock->dev.dev, "Failed to get event > > data\n"); > > return; > > } > > + > > + obj = data.pointer; > > + if (!obj) { > > + dev_warn(&wblock->dev.dev, "Event contains not event > > data\n"); > > I just noticed that this should have been "Event contains no event data\n". > Should i send > another patch? Hi Armin, As I was doing some history manipulation anyway as is, I tweaked it directly in the history. While doing the conflict resolution because of that small change I realized the wording got corrected in the latter patch anyway so it was quite harmless but it's now correct in both commits in review-ilpo branch.
Am 20.02.24 um 10:19 schrieb Ilpo Järvinen: > On Tue, 20 Feb 2024, Armin Wolf wrote: > >> Am 19.02.24 um 12:59 schrieb Armin Wolf: >> >>> WMI event drivers which do not have no_notify_data set expect >>> that each WMI event contains valid data. Evaluating _WED however >>> might return no data, which can cause issues with such drivers. >>> >>> Fix this by validating that evaluating _WED did return data. >>> >>> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >>> --- >>> drivers/platform/x86/wmi.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c >>> index 8fb90b726f50..d0fe8153f803 100644 >>> --- a/drivers/platform/x86/wmi.c >>> +++ b/drivers/platform/x86/wmi.c >>> @@ -1210,6 +1210,7 @@ static void wmi_notify_driver(struct wmi_block >>> *wblock) >>> { >>> struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); >>> struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; >>> + union acpi_object *obj = NULL; >>> acpi_status status; >>> >>> if (!driver->no_notify_data) { >>> @@ -1218,12 +1219,18 @@ static void wmi_notify_driver(struct wmi_block >>> *wblock) >>> dev_warn(&wblock->dev.dev, "Failed to get event >>> data\n"); >>> return; >>> } >>> + >>> + obj = data.pointer; >>> + if (!obj) { >>> + dev_warn(&wblock->dev.dev, "Event contains not event >>> data\n"); >> I just noticed that this should have been "Event contains no event data\n". >> Should i send >> another patch? > Hi Armin, > > As I was doing some history manipulation anyway as is, I tweaked it > directly in the history. While doing the conflict resolution because of > that small change I realized the wording got corrected in the latter patch > anyway so it was quite harmless but it's now correct in both commits in > review-ilpo branch. > Thank you!
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 8fb90b726f50..d0fe8153f803 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -1210,6 +1210,7 @@ static void wmi_notify_driver(struct wmi_block *wblock) { struct wmi_driver *driver = drv_to_wdrv(wblock->dev.dev.driver); struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *obj = NULL; acpi_status status; if (!driver->no_notify_data) { @@ -1218,12 +1219,18 @@ static void wmi_notify_driver(struct wmi_block *wblock) dev_warn(&wblock->dev.dev, "Failed to get event data\n"); return; } + + obj = data.pointer; + if (!obj) { + dev_warn(&wblock->dev.dev, "Event contains not event data\n"); + return; + } } if (driver->notify) - driver->notify(&wblock->dev, data.pointer); + driver->notify(&wblock->dev, obj); - kfree(data.pointer); + kfree(obj); } static int wmi_notify_device(struct device *dev, void *data)
WMI event drivers which do not have no_notify_data set expect that each WMI event contains valid data. Evaluating _WED however might return no data, which can cause issues with such drivers. Fix this by validating that evaluating _WED did return data. Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/platform/x86/wmi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) -- 2.39.2