wine-devel

wine-devel@winehq.org

9 participants
84431 discussions

Re: [PATCH] ntsync: Set the permissions to be 0666
by Elizabeth Figura 18 Feb '25

18 Feb '25

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.

1 0

Re: [PATCH] ntsync: Set the permissions to be 0666
by Elizabeth Figura 15 Feb '25

15 Feb '25

On Friday, 14 February 2025 19:03:33 CST Darrick J. Wong wrote: > On Fri, Feb 14, 2025 at 04:15:25PM -0600, Elizabeth Figura wrote: > > On Friday, 14 February 2025 12:45:39 CST Darrick J. Wong wrote: > > > On Fri, Feb 14, 2025 at 12:13:03PM -0600, Elizabeth Figura wrote: > > > > On Friday, 14 February 2025 07:06:20 CST Greg Kroah-Hartman wrote: > > > > > On Fri, Feb 14, 2025 at 12:28:00PM +0000, Mike Lothian wrote: > > > > > > This allows ntsync to be usuable by non-root processes out of the box > > > > > > > > > > Are you sure you need/want that? If so, why? How did existing testing > > > > > not ever catch this? > > > > > > > > Hi, sorry, this is of course my fault. > > > > > > > > We do need /dev/ntsync to be openable from user space for it to be > > > > useful. I'm not sure what the most "correct" permissions are to have > > > > in this case (when we don't specifically need read or write), but I > > > > don't think I see a reason not to just set to 666 or 444. > > > > > > > > I originally assumed that the right way to do this was not to set the > > > > mode on the kernel file but rather through udev; I believe I was using > > > > the code for /dev/loop-control or /dev/fuse as an example, which both > > > > do that. So I (and others who tested) had just manually set up udev > > > > rules for this, with the eventual intent of adding a default rule to > > > > systemd like the others. I only recently realized that doing something > > > > like this patch is possible and precedented. > > > > > > > > I don't know what the best way to address this is, but this is > > > > certainly the simplest. > > > > > > Paranoid defaults in the kernel, and then a udev rule to relax the mode > > > at runtime. You could also have logind scripts to add add per-user > > > allow acls to the device file at user session set up time... or however > > > it is that /dev/sr0 has me on the allow list. I'm not sure how that > > > happens exactly, but it works smoothly. > > > > > > I get far less complaining about relaxing posture than tightening it > > > (==breaking things) after the fact. > > > > FWIW, it may be worth stressing that this is not a hardware device in > > any sense, it's a software driver that only lives in a char device > > (and dedicated module) for the sake of isolating the code. I can't > > imagine any reason to control access per-user, although my experience > > may not be enough to grant such imagination. > > Oh, I'm aware that ntsync is a driver for a software "device" that > implements various Windows APIs and isn't real hardware. :) > > But, you might want prevent non-root systemd services (e.g. avahi) from > being able to access /dev/ntsync if, say, someone breaches that, while > at the same time allowing access to (say) logged-in users who can run > Wine. I see the idea, though I don't know if it's applicable in this case—the individual ntsync file descriptions are also supposed to be isolated from one another, so even a rogue avahi would still have another barrier towards compromising a Wine process. Of course you may be aware of that and be advocating for the more barriers the better. I can't say what systemd will actually prefer in this case, but they do seem to have requested that the kernel change it... [1] [1] https://github.com/systemd/systemd/pull/36384

1 0

Re: [PATCH] ntsync: Set the permissions to be 0666
by Elizabeth Figura 14 Feb '25

14 Feb '25

On Friday, 14 February 2025 12:45:39 CST Darrick J. Wong wrote: > On Fri, Feb 14, 2025 at 12:13:03PM -0600, Elizabeth Figura wrote: > > On Friday, 14 February 2025 07:06:20 CST Greg Kroah-Hartman wrote: > > > On Fri, Feb 14, 2025 at 12:28:00PM +0000, Mike Lothian wrote: > > > > This allows ntsync to be usuable by non-root processes out of the box > > > > > > Are you sure you need/want that? If so, why? How did existing testing > > > not ever catch this? > > > > Hi, sorry, this is of course my fault. > > > > We do need /dev/ntsync to be openable from user space for it to be > > useful. I'm not sure what the most "correct" permissions are to have > > in this case (when we don't specifically need read or write), but I > > don't think I see a reason not to just set to 666 or 444. > > > > I originally assumed that the right way to do this was not to set the > > mode on the kernel file but rather through udev; I believe I was using > > the code for /dev/loop-control or /dev/fuse as an example, which both > > do that. So I (and others who tested) had just manually set up udev > > rules for this, with the eventual intent of adding a default rule to > > systemd like the others. I only recently realized that doing something > > like this patch is possible and precedented. > > > > I don't know what the best way to address this is, but this is > > certainly the simplest. > > Paranoid defaults in the kernel, and then a udev rule to relax the mode > at runtime. You could also have logind scripts to add add per-user > allow acls to the device file at user session set up time... or however > it is that /dev/sr0 has me on the allow list. I'm not sure how that > happens exactly, but it works smoothly. > > I get far less complaining about relaxing posture than tightening it > (==breaking things) after the fact. FWIW, it may be worth stressing that this is not a hardware device in any sense, it's a software driver that only lives in a char device (and dedicated module) for the sake of isolating the code. I can't imagine any reason to control access per-user, although my experience may not be enough to grant such imagination. The only actual risk is a bug in the code itself—which is always possible—but at that point you'd presumably just want to disable it at build time or something similar.

1 0

Re: [PATCH] ntsync: Set the permissions to be 0666
by Elizabeth Figura 14 Feb '25

14 Feb '25

On Friday, 14 February 2025 07:06:20 CST Greg Kroah-Hartman wrote: > On Fri, Feb 14, 2025 at 12:28:00PM +0000, Mike Lothian wrote: > > This allows ntsync to be usuable by non-root processes out of the box > > Are you sure you need/want that? If so, why? How did existing testing > not ever catch this? Hi, sorry, this is of course my fault. We do need /dev/ntsync to be openable from user space for it to be useful. I'm not sure what the most "correct" permissions are to have in this case (when we don't specifically need read or write), but I don't think I see a reason not to just set to 666 or 444. I originally assumed that the right way to do this was not to set the mode on the kernel file but rather through udev; I believe I was using the code for /dev/loop-control or /dev/fuse as an example, which both do that. So I (and others who tested) had just manually set up udev rules for this, with the eventual intent of adding a default rule to systemd like the others. I only recently realized that doing something like this patch is possible and precedented. I don't know what the best way to address this is, but this is certainly the simplest.

1 0

Wine staging 10.1 release
by Alistair Leslie-Hughes 09 Feb '25

09 Feb '25

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary since last release * Rebased to current wine 10.1 (361 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * None Removed (No longer required). * None Added: * [53644] vbscript: class single line multivar. * [56464] vbscript: Implement Join * [52769] mf: Add HTTP/HTTPS IMFByteStream implementation. * [26302] richedit20: Implement IRichEditOle::ImportDataObject * [53620] err:ole:com_get_class_object class IEnumObjects not registered. * [56891] Visual novel Mebae (Tanuki Soft) crashes on startup * [51139] d2d1: Implement drawing of ID2D1GeometryGroup * [56280] oleaut32: VarAnd use VT_I4 when converting VT_BSTR Updated: * vkd3d-latest Where can you help * Run Steam/Battle.net/GOG/UPlay/Epic * Test your favorite game. * Test your favorite applications. * Improve staging patches and get them accepted upstream. * Suggest patches to be included in staging. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

1 0

FOSDEM
by Huw Davies 30 Jan '25

30 Jan '25

Hi folks, Many CodeWeavers folks will be in Brussels this weekend attending FOSDEM. If anybody else is around, or near Brussels, please feel free to either drop me an email or simply turn up to the BrewDog on Friday or Saturday evenings from 17:00. Many of us will be in Brussels Thursday and Sunday evenings too, but our exact location is less certain; if you let me know you’re interested, I can keep you in the loop. Sorry for the late notice! Huw

1 0

Wine staging 10.0 release
by Alistair Leslie-Hughes 21 Jan '25

21 Jan '25

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary since last release * Rebased to current wine 10.0 (346 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * None Removed (No longer required). * None Added: * None Updated: * vkd3d-latest Where can you help * Run Steam/Battle.net/GOG/UPlay/Epic * Test your favorite game. * Test your favorite applications. * Improve staging patches and get them accepted upstream. * Suggest patches to be included in staging. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

1 0

10.0 release notes, please review
by Alexandre Julliard 20 Jan '25

20 Jan '25

Folks, Here are the draft release notes for Wine 10.0. Please review them and let me know if anything is incorrect or missing. Thanks to Jacek and Rémi for their help! The final 10.0 should be out in a couple of days. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine staging 10.0-rc6 release
by Alistair Leslie-Hughes 18 Jan '25

18 Jan '25

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary since last release * Rebased to current wine 10.0-rc6 (348 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * None Removed (No longer required). * None Added: * None Updated: * vkd3d-latest Where can you help * Run Steam/Battle.net/GOG/UPlay/Epic * Test your favorite game. * Test your favorite applications. * Improve staging patches and get them accepted upstream. * Suggest patches to be included in staging. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

1 0

[PATCH] ntsync: Fix reference leaks in the remaining create ioctls.
by Elizabeth Figura 16 Jan '25

16 Jan '25

When ntsync_obj_get_fd() fails, we free the ntsync object but forget to drop the "file" member. This was fixed for semaphores in 0e7d523b5f7a23b1dc6ceceb04e31a60e9e3321d, but that commit did not fix the similar leak for events and mutexes, since they were part of patches not yet in the mainline kernel. Fix those cases. Fixes: 5bc2479a3585b "ntsync: Introduce NTSYNC_IOC_CREATE_MUTEX." Fixes: 4c7404b9c2b57 "ntsync: Introduce NTSYNC_IOC_CREATE_EVENT." Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> --- drivers/misc/ntsync.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index b6441e978974..055395cde42b 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -781,7 +781,7 @@ static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp) mutex->u.mutex.owner = args.owner; fd = ntsync_obj_get_fd(mutex); if (fd < 0) - kfree(mutex); + ntsync_free_obj(mutex); return fd; } @@ -802,7 +802,7 @@ static int ntsync_create_event(struct ntsync_device *dev, void __user *argp) event->u.event.signaled = args.signaled; fd = ntsync_obj_get_fd(event); if (fd < 0) - kfree(event); + ntsync_free_obj(event); return fd; } base-commit: 0e7d523b5f7a23b1dc6ceceb04e31a60e9e3321d -- 2.45.2

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel