Re: [PATCH 3/6] server: Implement changing the label of a security descriptor.

[Alexandre Julliard]

Matteo Bruni  writes:
> From: Michael Müller
> Signed-off-by: Matteo Bruni
>
> dlls/advapi32/tests/security.c | 153 ++++++++++++++++++++++++++++++++++++++++-
>  dlls/ntdll/sec.c               |   3 +-
>  server/handle.c                | 130 +++++++++++++++++++++++++++++++++-
>  3 files changed, 282 insertions(+), 4 deletions(-)

It doesn't work here:

../../../tools/runtest -q -P wine -T ../../.. -M advapi32.dll -p advapi32_test.exe.so security && touch security.ok
security.c:2065: Tests skipped: Well known SID 38 not implemented
security.c:2065: Tests skipped: Well known SID 39 not implemented
security.c:2065: Tests skipped: Well known SID 40 not implemented
security.c:2065: Tests skipped: Well known SID 41 not implemented
security.c:2065: Tests skipped: Well known SID 42 not implemented
security.c:2065: Tests skipped: Well known SID 43 not implemented
security.c:2065: Tests skipped: Well known SID 44 not implemented
security.c:2065: Tests skipped: Well known SID 45 not implemented
security.c:2065: Tests skipped: Well known SID 46 not implemented
security.c:2065: Tests skipped: Well known SID 47 not implemented
security.c:2065: Tests skipped: Well known SID 48 not implemented
security.c:2065: Tests skipped: Well known SID 49 not implemented
security.c:2065: Tests skipped: Well known SID 50 not implemented
security.c:2065: Tests skipped: Well known SID 62 not implemented
security.c:2065: Tests skipped: Well known SID 63 not implemented
security.c:2065: Tests skipped: Well known SID 64 not implemented
security.c:2065: Tests skipped: Well known SID 65 not implemented
security.c:2065: Tests skipped: Well known SID 70 not implemented
security.c:2065: Tests skipped: Well known SID 71 not implemented
security.c:2065: Tests skipped: Well known SID 72 not implemented
security.c:2065: Tests skipped: Well known SID 73 not implemented
security.c:2065: Tests skipped: Well known SID 74 not implemented
security.c:2065: Tests skipped: Well known SID 75 not implemented
security.c:2065: Tests skipped: Well known SID 76 not implemented
security.c:2065: Tests skipped: Well known SID 77 not implemented
security.c:2065: Tests skipped: Well known SID 78 not implemented
security.c:2065: Tests skipped: Well known SID 79 not implemented
security.c:2065: Tests skipped: Well known SID 81 not implemented
security.c:2065: Tests skipped: Well known SID 82 not implemented
wine: Unhandled page fault on read access to 0x00000004 at address 0x7ed5a0bf (thread 0047), starting debugger...
Unhandled exception: page fault on read access to 0x00000004 in 32-bit code (0x7ed5a0bf).
Register dump:
 CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
 EIP:7ed5a0bf ESP:0033f9ac EBP:0033fb88 EFLAGS:00010206(  R- --  I   - -P- )
 EAX:00000000 EBX:7ed83000 ECX:0000001f EDX:00000400
 ESI:00133838 EDI:7ed73e9f
Stack dump:
0x0033f9ac:  0000007a 00000000 00000000 7ed79a04
0x0033f9bc:  7ed74d52 7ed7996c 7ed79918 0033fa44
0x0033f9cc:  00000001 7ed79940 00000001 7ed84b58
0x0033f9dc:  7ed79994 7ed798e8 001336e0 7ed74cc0
0x0033f9ec:  7ed74cb2 7ed74ca0 0033fa40 0033fa30
0x0033f9fc:  0033fa2c 7ed7980c 7ed797dc 7ed79a90
Backtrace:
=>0 0x7ed5a0bf test_AddMandatoryAce+0xf9f() [/home/julliard/wine/wine/dlls/advapi32/tests/security.c:6348] in advapi32_test (0x0033fb88)
  1 0x7ed625fa func_security+0x24a9() [/home/julliard/wine/wine/dlls/advapi32/tests/security.c:6764] in advapi32_test (0x0033fd48)
  2 0x7ed2596d main+0x25c(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/dlls/advapi32/tests/../../../include/wine/test.h:589] in advapi32_test (0x0033fdf8)
  3 0x7ed6df9b __wine_spec_exe_entry+0x4a(peb=<couldn't compute location>) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in advapi32_test (0x0033fe38)
  4 0x7b45e79c call_process_entry+0xb() in kernel32 (0x0033fe58)
  5 0x7b45f6d9 start_process+0x68(entry=<couldn't compute location>) [/home/julliard/wine/wine/dlls/kernel32/process.c:1116] in kernel32 (0x0033fe98)
  6 0x7bc7f23c call_thread_func_wrapper+0xb() in ntdll (0x0033feb8)
  7 0x7bc81ff9 call_thread_func+0xa8(entry=0x7b45f670, arg=0x7ed6df50, frame=0x33ffc8) [/home/julliard/wine/wine/dlls/ntdll/signal_i386.c:2759] in ntdll (0x0033ffa8)
  8 0x7bc7f21a call_thread_entry_point+0x11() in ntdll (0x0033ffc8)
  9 0x7bc54023 start_process+0x12(arg=0xff871728) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3083] in ntdll (0x0033ffe8)
  10 0xf75938cd __x86.get_pc_thunk.di+0x2a() in libwine.so.1 (0x00000000)
  11 0xf7593a30 wine_switch_to_stack+0x1f(func=0x7bc54010, arg=0xff871728, stack=0x340000) [/home/julliard/wine/wine/libs/wine/port.c:77] in libwine.so.1 (0xff8716f8)
  12 0x7bc59cde LdrInitializeThunk+0x22d(kernel_start=<couldn't compute location>, unknown2=<couldn't compute location>, unknown3=<couldn't compute location>, unknown4=<couldn't compute location>) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3144] in ntdll (0xff871748)
  13 0x7b46563f __wine_kernel_init+0xa4e() [/home/julliard/wine/wine/dlls/kernel32/process.c:1310] in kernel32 (0xff872638)
  14 0x7bc5ab33 __wine_process_init+0x152() [/home/julliard/wine/wine/dlls/ntdll/loader.c:3353] in ntdll (0xff8726a8)
  15 0xf7591b33 wine_init+0x2a2(argc=0x3, argv=0xff872bf4, error="", error_size=0x400) [/home/julliard/wine/wine/libs/wine/loader.c:974] in libwine.so.1 (0xff8726f8)
  16 0x7c000b58 main+0x97(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/loader/main.c:258] in <wine-loader> (0xff872b48)
  17 0xf739b276 __libc_start_main+0xf5() in libc.so.6 (0x00000000)
0x7ed5a0bf test_AddMandatoryAce+0xf9f [/home/julliard/wine/wine/dlls/advapi32/tests/security.c:6348] in advapi32_test: movzwl	0x4(%eax),%edx
6348	    ok(!sacl->AceCount, "SACL contains an unexpected ACE count %u\n", sacl->AceCount);

[PATCH 3/6] server: Implement changing the label of a security descriptor.

[Sebastian Lackner]

On 13.06.2017 21:00, Alexandre Julliard wrote:
> Matteo Bruni  writes:
> > From: Michael Müller
> > Signed-off-by: Matteo Bruni
> >
> > dlls/advapi32/tests/security.c | 153 ++++++++++++++++++++++++++++++++++++++++-
> >  dlls/ntdll/sec.c               |   3 +-
> >  server/handle.c                | 130 +++++++++++++++++++++++++++++++++-
> >  3 files changed, 282 insertions(+), 4 deletions(-)
>
> It doesn't work here:
> [...]

I can't test it right now, but I suspect that this is caused by a change done by Matteo Bruni in patch 2.
> - if (!count) return 1;
> -

This wasn't present yet in Michael Müllers original patchset.

Best regards,
Sebastian

[PATCH 3/6] server: Implement changing the label of a security descriptor.

[Sebastian Lackner]

On 13.06.2017 22:12, Sebastian Lackner wrote:
> On 13.06.2017 21:00, Alexandre Julliard wrote:
> > Matteo Bruni  writes:
> > > From: Michael Müller
> > > Signed-off-by: Matteo Bruni
> > >
> > > dlls/advapi32/tests/security.c | 153 ++++++++++++++++++++++++++++++++++++++++-
> > >  dlls/ntdll/sec.c               |   3 +-
> > >  server/handle.c                | 130 +++++++++++++++++++++++++++++++++-
> > >  3 files changed, 282 insertions(+), 4 deletions(-)
> >
> > It doesn't work here:
> > [...]
>
> I can't test it right now, but I suspect that this is caused by a change done by Matteo Bruni in patch 2.
> > - if (!count) return 1;
> > -
>
> This wasn't present yet in Michael Müllers original patchset.
>
> Best regards,
> Sebastian

It is indeed caused by this specific change, so it probably makes sense to hold back patch 2 aswell.
@Matteo: Was there any specific reason for this change?

Best regards,
Sebastian

[PATCH 3/6] server: Implement changing the label of a security descriptor.

[Matteo Bruni]

2017-06-13 22:34 GMT+02:00 Sebastian Lackner :
> On 13.06.2017 22:12, Sebastian Lackner wrote:
> > On 13.06.2017 21:00, Alexandre Julliard wrote:
> > > Matteo Bruni  writes:
> > > > From: Michael Müller
> > > > Signed-off-by: Matteo Bruni
> > > >
> > > > dlls/advapi32/tests/security.c | 153 ++++++++++++++++++++++++++++++++++++++++-
> > > >  dlls/ntdll/sec.c               |   3 +-
> > > >  server/handle.c                | 130 +++++++++++++++++++++++++++++++++-
> > > >  3 files changed, 282 insertions(+), 4 deletions(-)
> > >
> > > It doesn't work here:
> > > [...]
> >
> > I can't test it right now, but I suspect that this is caused by a change done by Matteo Bruni in patch 2.
> > > - if (!count) return 1;
> > > -
> >
> > This wasn't present yet in Michael Müllers original patchset.
> >
> > Best regards,
> > Sebastian
>
> It is indeed caused by this specific change, so it probably makes sense to hold back patch 2 aswell.
> @Matteo: Was there any specific reason for this change?
>
> Best regards,
> Sebastian

No, it was a last-minute change (I think it was to avoid malloc(0) but
clearly there's no such danger) and apparently I didn't retest the
patches after adding it. So double fail.

I'll send the fixed and retested patches soon.