[PATCH] user32: Fix NULL dereference in UnregisterDeviceNotification

[Arkadiusz Hiler]

On Wed, May 20, 2020 at 09:01:21AM -0500, Zebediah Figura wrote:
> On 5/20/20 7:14 AM, Zhiyi Zhang wrote:
> > Hi Arkadiusz,
> > 
> > Checking NULL is usually fine. But I think you should add the tests to sechost.
> > There are also
> > https://github.com/ValveSoftware/wine/commit/bbcd2686d599adf6c5cc8e8466ade0b2c2e5f38a
> > https://github.com/ValveSoftware/wine/commit/2d5d0a50652e13254fef3d13df14d3b01efda938
> > 
> > I haven't look at if I_ScUnregisterDeviceNotification() is already fully implemented.
> > But if it is not, it would be more useful to get it upstream rather than simply checking NULL.
> > If checking NULL is enough to fix a real world application and you think upstreamming
> > the work is too much for you then a simple NULL fix is also fine.
> 
> Personally, I think the test makes sense where it is.

Hey,

Sorry for late reply, I subscribe to about a dozen of mailing lists and
I am used that people in the discussion are explicitly CCed. I'll adjust
my filtering for wine-devel.

I am still wrapping my head around some of the things here. As far as I
undersand I_ScUnregisterDeviceNotification() is internal and is not
exposed by any header. UnregisterDeviceNotification() it the call that a
lot of programs are usining through the winuser.h, and therefore should
have conformance tests on it's own.

I_ScUnregisterDeviceNotification() may be called by some .exes though
anyway, so it should have it's own set of conformance tests too.

Is this correct?

> On the other hand, it'd be nice to mention in the commit message what
> application is fixed by this.

It's the Glorious Model O control software. It won't work but at least
this fixes the crash.

I'll resend my patch with that mentioned + Roman's bug linked and CC
you. Thanks!

-- 
Cheers,
Arek