February 2025 - Wine-Devel - list.winehq.org

Re: [PATCH] ntsync: Check wait count based on byte size.

by Elizabeth Figura

On Wednesday, 19 February 2025 08:25:27 CST Greg Kroah-Hartman wrote: > On Tue, Feb 18, 2025 at 06:19:20PM -0600, Elizabeth Figura wrote: > > GCC versions below 13 incorrectly detect the copy size as being static and too > > small to fit in the "fds" array. Work around this by explicitly calculating the > > size and returning EINVAL based on that, instead of based on the object count. > > > > Reported-by: kernel test robot <lkp(a)intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202502072019.LYoCR9bF-lkp(a)intel.com/ > > Suggested-by: Arnd Bergmann <arnd(a)arndb.de> > > > > -- > > > > Suggested-by as per Arnd's request, but the only thing I changed was preserving > > array_size() [as noted by Geert in the linked thread]. I tested and found no > > regressions. > > You forgot to sign-off on this commit :( > > Can you resend it please with that fixed? Done, thanks. I used to have format.signOff=true but lost that somewhere...

Feb. 20, 2025

Re: [PATCH v2] ntsync: Check wait count based on byte size.

by Elizabeth Figura

On Thursday, 20 February 2025 13:23:34 CST Elizabeth Figura wrote: > GCC versions below 13 incorrectly detect the copy size as being static and too > small to fit in the "fds" array. Work around this by explicitly calculating the > size and returning EINVAL based on that, instead of based on the object count. > > Reported-by: kernel test robot <lkp(a)intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202502072019.LYoCR9bF-lkp(a)intel.com/ > Suggested-by: Arnd Bergmann <arnd(a)arndb.de> > Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> > Oops, I forgot to include a reference to v1: https://lore.kernel.org/all/20250219001920.210847-1-zfigura(a)codeweavers.c…

Feb. 20, 2025

[PATCH v2] ntsync: Check wait count based on byte size.

by Elizabeth Figura

GCC versions below 13 incorrectly detect the copy size as being static and too small to fit in the "fds" array. Work around this by explicitly calculating the size and returning EINVAL based on that, instead of based on the object count. Reported-by: kernel test robot <lkp(a)intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202502072019.LYoCR9bF-lkp(a)intel.com/ Suggested-by: Arnd Bergmann <arnd(a)arndb.de> Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> -- Suggested-by as per Arnd's request, but the only thing I changed was preserving array_size() [as noted by Geert in the linked thread]. I tested and found no regressions. v2: Add missing sign-off --- drivers/misc/ntsync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index 586b86243e1d..24cb2f71186a 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -873,6 +873,7 @@ static int setup_wait(struct ntsync_device *dev, { int fds[NTSYNC_MAX_WAIT_COUNT + 1]; const __u32 count = args->count; + size_t size = array_size(count, sizeof(fds[0])); struct ntsync_q *q; __u32 total_count; __u32 i, j; @@ -880,15 +881,14 @@ static int setup_wait(struct ntsync_device *dev, if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME)) return -EINVAL; - if (args->count > NTSYNC_MAX_WAIT_COUNT) + if (size >= sizeof(fds)) return -EINVAL; total_count = count; if (args->alert) total_count++; - if (copy_from_user(fds, u64_to_user_ptr(args->objs), - array_size(count, sizeof(*fds)))) + if (copy_from_user(fds, u64_to_user_ptr(args->objs), size)) return -EFAULT; if (args->alert) fds[count] = args->alert; -- 2.47.2

Feb. 20, 2025

Re: Powrprof patches

by Alexandre Julliard

Stefan Dösinger <stefandoesinger(a)gmail.com> writes: >> Am 19.02.2025 um 23:38 schrieb robert lippmann <robert.lippmann.development(a)gmail.com>: >> >> Some more follow up questions: >> >> 1) Since there are a lot of new functions added, should I do an >> initial patch with winedump producing skeleton code, and have each >> function just return ERROR_CALL_NOT_IMPLEMENTED? And then do follow >> on patches to implement? Or just do the implementation at the same >> time? From a review standpoint, I imagine the first approach is >> better. > > The first approach is generally better. Make sure that every function writes a FIXME to make it obvious a stub is called. Unless it's a COM interface, you don't want to add skeleton code. Just list the functions as stub in the spec file, and implement only the ones that are called by the application. -- Alexandre Julliard julliard(a)winehq.org

Feb. 20, 2025

Re: Powrprof patches

by Stefan Dösinger

> Am 19.02.2025 um 23:38 schrieb robert lippmann <robert.lippmann.development(a)gmail.com>: > > Some more follow up questions: > > 1) Since there are a lot of new functions added, should I do an initial patch with winedump producing skeleton code, and have each function just return ERROR_CALL_NOT_IMPLEMENTED? And then do follow on patches to implement? Or just do the implementation at the same time? From a review standpoint, I imagine the first approach is better. The first approach is generally better. Make sure that every function writes a FIXME to make it obvious a stub is called. > 2) A bunch of power related GUIDs are defined in winnt.h in the Windows SDK. It seems like they should be in the power headers. Should I follow the SDK? Yeah, follow the SDK. At least in theory our headers should be a drop in replacement for Microsoft's, although in practice that doesn't get tested much. > 3) For new files I’ll need to create, do I set the copyright name to me? Or Alexandre? Yourself

Feb. 20, 2025

Re: Powrprof patches

by robert lippmann

Some more follow up questions: 1) Since there are a lot of new functions added, should I do an initial patch with winedump producing skeleton code, and have each function just return ERROR_CALL_NOT_IMPLEMENTED? And then do follow on patches to implement? Or just do the implementation at the same time? From a review standpoint, I imagine the first approach is better. 2) A bunch of power related GUIDs are defined in winnt.h in the Windows SDK. It seems like they should be in the power headers. Should I follow the SDK? 3) For new files I’ll need to create, do I set the copyright name to me? Or Alexandre? Thanks, Rob

Feb. 19, 2025

[PATCH] ntsync: Check wait count based on byte size.

by Elizabeth Figura

GCC versions below 13 incorrectly detect the copy size as being static and too small to fit in the "fds" array. Work around this by explicitly calculating the size and returning EINVAL based on that, instead of based on the object count. Reported-by: kernel test robot <lkp(a)intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202502072019.LYoCR9bF-lkp(a)intel.com/ Suggested-by: Arnd Bergmann <arnd(a)arndb.de> -- Suggested-by as per Arnd's request, but the only thing I changed was preserving array_size() [as noted by Geert in the linked thread]. I tested and found no regressions. --- drivers/misc/ntsync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index 586b86243e1d..24cb2f71186a 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -873,6 +873,7 @@ static int setup_wait(struct ntsync_device *dev, { int fds[NTSYNC_MAX_WAIT_COUNT + 1]; const __u32 count = args->count; + size_t size = array_size(count, sizeof(fds[0])); struct ntsync_q *q; __u32 total_count; __u32 i, j; @@ -880,15 +881,14 @@ static int setup_wait(struct ntsync_device *dev, if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME)) return -EINVAL; - if (args->count > NTSYNC_MAX_WAIT_COUNT) + if (size >= sizeof(fds)) return -EINVAL; total_count = count; if (args->alert) total_count++; - if (copy_from_user(fds, u64_to_user_ptr(args->objs), - array_size(count, sizeof(*fds)))) + if (copy_from_user(fds, u64_to_user_ptr(args->objs), size)) return -EFAULT; if (args->alert) fds[count] = args->alert; -- 2.47.2

Feb. 19, 2025

Re: [PATCH] ntsync: Set the permissions to be 0666

by Elizabeth Figura

On Friday, 14 February 2025 06:28:00 CST Mike Lothian wrote: > This allows ntsync to be usuable by non-root processes out of the box > > Signed-off-by: Mike Lothian <mike(a)fireburn.co.uk> > --- > drivers/misc/ntsync.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c > index 055395cde42b..586b86243e1d 100644 > --- a/drivers/misc/ntsync.c > +++ b/drivers/misc/ntsync.c > @@ -1208,6 +1208,7 @@ static struct miscdevice ntsync_misc = { > .minor = MISC_DYNAMIC_MINOR, > .name = NTSYNC_NAME, > .fops = &ntsync_fops, > + .mode = 0666, // Setting file permissions to 0666 > }; > > module_misc_device(ntsync_misc); > Reviewed-by: Elizabeth Figura <zfigura(a)codeweavers.com> -- The comment seems rather redundant, but otherwise this is correct and prudent.

Feb. 18, 2025

Re: Powrprof patches

by Elizabeth Figura

On Monday, February 17, 2025 3:52:52 AM Central Standard Time robert lippmann wrote: > While I could just hardcode the GUIDs and indexes for the default power > schemes in Windows, it seems like the more “correct” solution is to > populate and manipulate the registry like Windows does. No, hardcoding things like that is typically fine.

Feb. 17, 2025

Re: Powrprof patches

by robert lippmann

While I could just hardcode the GUIDs and indexes for the default power schemes in Windows, it seems like the more “correct” solution is to populate and manipulate the registry like Windows does. And, as I don’t plan on fully implementing all the calls, it might make it easier down the road for someone who might. > On Feb 17, 2025, at 3:51 AM, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > > Am Montag, 17. Februar 2025, 00:45:32 Ostafrikanische Zeit schrieb robert > lippmann: >> 1) Default power profiles are stored in the registry under >> HKLM\System\CurrentControlSet\Control\Power. The subtree is pretty huge >> (about a 1.5m .reg file when I exported it. > > What's your ultimate goal? I guess you have an application that tries to read > and/or change power settings. > > If the application is merely reading to e.g. figure out if it is running on a > laptop powered by battery you probably don't need most of the registry values, > > If you want to make a power management tool work then you are in for a lot of > work. Powerprof would have to talk to lower level Linux components to actually > apply changes to the hardware settings - that's going to be tricky.

Feb. 17, 2025