Re: [PATCH] kernel32: CreateDirectory shouldn't return ERROR_ACCESS_DENIED for the root of the drive.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Alexandre Julliard  wrote:
> > > > > > >> > According to the testbot results CreateDirectory("C:\", NULL) fails
> > > > > > >> > with ERROR_ACCESS_DENIED for not administrators. However with UAC enabled
> > > > > > >> > and not and administrator account I get ERROR_ALREADY_EXISTS in that case
> > > > > > >> > with Windows 7 64-bit running on real hardware. Moreover, Wine doesn't
> > > > > > >> > really perform any access checks in that case and blindly assumes that
> > > > > > >> > returning STATUS_ACCESS_DENIED is correct behaviour for the drive's root:
> > > > > > >> > dlls/ntdll/directory.c,lookup_unix_name().
> > > > > > >> >
> > > > > > >> > This patch fixes an application that can't find its data files because
> > > > > > >> > after it receives ERROR_ACCESS_DENIED it stops further directory traversing.
> > > > > > >> 
> > > > > > >> What app is that? Does it fail on Windows when not administrator?
> > > > > > >
> > > > > > > The application doesn't fail on Windows, and as I mentioned above under
> > > > > > > a not administrator account and UAC enabled I don't get ERROR_ACCESS_DENIED
> > > > > > > error with the tests included in the patch.
> > > > > >
> > > > > > Your tests get ERROR_ACCESS_DENIED on every single testbot vm except
> > > > > > w8adm, so that's not very convincing... Is the app going to fail on all
> > > > > > these vms?
> > > > >
> > > > > I'd guess if the CreateDirectory("C:") returns ERROR_ACCESS_DENIED
> > > > > then the app would fail. It's not clear how the VMs are configured
> > > > > and why I don't get ERROR_ACCESS_DENIED on real hardware under a not
> > > > > admin account, but it should be pretty obvious that since Wine doesn't
> > > > > perform any real administrator access checks, and if it would the checks
> > > > > should be done on the server side, ntdll checks shouldn't return access
> > > > > denied error.
> > > >
> > > > It's also pretty obvious that this check was added for a reason, so it
> > > > would need a more convincing argument to remove it.
> > >
> > > Do you recall the reason why that check was added? I can't find any
> > > specific test case for this behaviour either.
> >
> > As far as I can tell, you added it ;-)
> >
> > https://source.winehq.org/git/wine.git/commit/d75aed2c92435e8ae4d5c260e31e81...
>
> This doesn't look right, especially without any reasonable explanation.
> Considering that now we have an application that depends on this, can
> that change be reverted?

I'd like to see some more convincing test cases.