[PATCHES] GPO support for the AD DC (vers 5)

Tue Nov 21 21:07:57 UTC 2017

Patches are finally upstream!

It's all a bit messier than desirable so that we could preserve some of
the history of the patches, but it's now finally in there after four years.

Cheers,

Garming

On 21/11/17 09:52, Garming Sam via samba-technical wrote:
> Hi,
>
> I've looked over the patches and I'm currently trying to send them
> upstream. It looks like with your 80 character patch, your tabs were set
> to 4 spaces instead of 8, so I basically just redid your patch.
>
> There seems to be a number of intermittent failures (which are
> pre-existing) which are blocking it. I'll probably take a look at those
> while I continue re-submitting.
>
>
> Cheers,
>
> Garming
>
> On 18/11/17 05:44, David Mulder wrote:
>> Have you looked at this Garming?
>>
>> On 11/09/2017 02:11 PM, Garming Sam wrote:
>>> Hi,
>>>
>>> It seems to pass the tests now. The tests also complain about the
>>> executable header in gpclass.py being unnecessary, so that probably
>>> needs to just be removed.
>>>
>>> I will start looking over the additional patches so that I can put my
>>> review on them.
>>>
>>>
>>> Cheers,
>>>
>>> Garming
>>>
>>> On 08/11/17 07:08, David Mulder wrote:
>>>> Here is the fix.
>>>> We also need to ensure we have a talloc stackframe in ads_connect().
>>>> Also, samba_gpoupdate needs to be executable or the samba4.gpo.apply
>>>> test fails to execute the script.
>>>> Again, I've also got the patches posted on my github repo:
>>>>
>>>> https://github.com/dmulder/samba/tree/garming-gpo-david-rebased
>>>>
>>>> Please re-test and hopefully let's get these pushed into master.
>>>>
>>>> On 11/06/2017 07:45 PM, Garming Sam wrote:
>>>>> Another failure here:
>>>>>
>>>>> [1246(6605)/2209 at 1h56m16s] samba4.gpo.apply(ad_dc:local)
>>>>> smbtorture 4.8.0pre1-DEVELOPERBUILD
>>>>> Using seed 1510022136
>>>>> no talloc stackframe at ../source3/libads/ldap.c:88, leaking memory
>>>>> PANIC (pid 15202): No talloc stackframe
>>>>> BACKTRACE: 17 stack frames:
>>>>>   #0 bin/shared/libsmbconf.so.0(log_stack_trace+0x1f) [0x2aafce37106e]
>>>>>   #1 bin/shared/libsmbconf.so.0(smb_panic_s3+0x6f) [0x2aafce370eb9]
>>>>>   #2 bin/shared/libsamba-util.so.0(smb_panic+0x28) [0x2aafcb72edf8]
>>>>>   #3 bin/shared/libsamba-util.so.0(_talloc_tos+0x10a) [0x2aafcb7323bc]
>>>>>   #4 bin/shared/private/libads-samba4.so(ldap_open_with_timeout+0xf6)
>>>>> [0x2aafce30539e]
>>>>>   #5 bin/shared/private/libads-samba4.so(ads_connect+0x5eb)
>>>>> [0x2aafce306ed8]
>>>>>   #6 bin/shared/private/libads-samba4.so(ads_connect_user_creds+0x2b)
>>>>> [0x2aafce30733f]
>>>>>   #7 bin/python/samba/gpo.so(+0x290a) [0x2aafce27e90a]
>>>>>   #8 python(PyEval_EvalFrameEx+0x118c) [0x524cdc]
>>>>>   #9 python(PyEval_EvalFrameEx+0xc9a) [0x5247ea]
>>>>>   #10 python(PyEval_EvalFrameEx+0xc9a) [0x5247ea]
>>>>>   #11 python() [0x567d14]
>>>>>   #12 python(PyRun_FileExFlags+0x92) [0x465bf4]
>>>>>   #13 python(PyRun_SimpleFileExFlags+0x2ee) [0x46612d]
>>>>>   #14 python(Py_Main+0xb5e) [0x466d92]
>>>>>   #15 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)
>>>>> [0x2aafca044f45]
>>>>>   #16 python() [0x577c2e]
>>>>> smb_panic(): calling panic action
>>>>> [/home/ubuntu/autobuild/b4818/samba/selftest/gdb_backtrace 15202]
>>>>> [Thread debugging using libthread_db enabled]
>>>>>
>>>>> Garming
>>>>>
>>>>>
>>>>> On 03/11/17 03:12, David Mulder wrote:
>>>>>> I've rebased Andrew's "Use talloc_stackframe() not talloc_tos() in
>>>>>> namequery.c" commit with this patch and pushed it to github:
>>>>>> https://github.com/dmulder/samba/tree/garming-gpo-david-rebased
>>>>>> I've also attached the rebased commit here.
>>>>>>
>>>>>> Garming, could you re-run your tests please?
>>>>>>
>>>>>> On 11/02/2017 07:49 AM, David Mulder via samba-technical wrote:
>>>>>>> This patch fixes the crash.
>>>>>>> We should free the frame from all return cases in resolve_name()
>>>>>>>
>>>>>>> On 11/01/2017 04:23 PM, Garming Sam wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Seen a failure in our test suites with the patches:
>>>>>>>>
>>>>>>>> [57(928)/2209 at 47m30s] samba.tests.tdb_util
>>>>>>>> [58(929)/2209 at 47m31s] samba.tests.tdb_util.python3
>>>>>>>> [59(930)/2209 at 47m31s] samba.tests.pam_winbind(local)(ad_member)
>>>>>>>> ERROR: Testsuite[samba.tests.pam_winbind(local)(ad_member)]
>>>>>>>> REASON: unable to set up environment ad_member - exiting
>>>>>>>>
>>>>>>>>
>>>>>>>> Freed frame ../source3/utils/net.c:917, expected
>>>>>>>> ../source3/libsmb/namequery.c:2871.
>>>>>>>> PANIC (pid 30133): Frame not freed in order.
>>>>>>>> BACKTRACE: 10 stack frames:
>>>>>>>>    #0 bin/shared/libsmbconf.so.0(log_stack_trace+0x1f)
>>>>>>>> [0x2b386416106e]
>>>>>>>>    #1 bin/shared/libsmbconf.so.0(smb_panic_s3+0x6f) [0x2b3864160eb9]
>>>>>>>>    #2 bin/shared/libsamba-util.so.0(smb_panic+0x28) [0x2b3863f60df8]
>>>>>>>>    #3 bin/shared/libsamba-util.so.0(+0x25fe3) [0x2b3863f63fe3]
>>>>>>>>    #4 bin/shared/private/libtalloc.so.2(+0x36a6) [0x2b38641bd6a6]
>>>>>>>>    #5 bin/shared/private/libtalloc.so.2(+0x3a2c) [0x2b38641bda2c]
>>>>>>>>    #6 bin/shared/private/libtalloc.so.2(_talloc_free+0x102)
>>>>>>>> [0x2b38641bece7]
>>>>>>>>    #7 ./bin/net(main+0xf24) [0x2b38637dc214]
>>>>>>>>    #8 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)
>>>>>>>> [0x2b38658a7f45]
>>>>>>>>    #9 ./bin/net(+0x246b9) [0x2b38637c26b9]
>>>>>>>>
>>>>>>>>
>>>>>>>> It looks like the talloc stackframe work hasn't quite been
>>>>>>>> completed.
>>>>>>>> There seems be another location which has not yet been included.
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>>
>>>>>>>> Garming
>>>>>>>>
>>>>>>>>
>>>>>>>> On 02/11/17 10:01, David Mulder via samba-technical wrote:
>>>>>>>>> Here is a patch to improve the formatting.
>>>>>>>>>
>>>>>>>>> On 11/01/2017 01:08 PM, Andrew Bartlett wrote:
>>>>>>>>>> On Thu, 2017-11-02 at 06:10 +1300, Andrew Bartlett via
>>>>>>>>>> samba-technical
>>>>>>>>>> wrote:
>>>>>>>>>>> On Wed, 2017-11-01 at 11:01 -0600, David Mulder wrote:
>>>>>>>>>>>> Oh, sorry. It actually all came through in a single email
>>>>>>>>>>>> (labeled
>>>>>>>>>>>> one
>>>>>>>>>>>> of thirty).
>>>>>>>>>>> Sorry, didn't scroll down ;-)
>>>>>>>>>>>
>>>>>>>>>>>> But it looks as though I missed a commit anyhow, since there
>>>>>>>>>>>> should be 31 commits, not 30.
>>>>>>>>>>>> Here is an attachment, and is posted to my github repo:
>>>>>>>>>>>> https://github.com/dmulder/samba/tree/garming-gpo-david-rebased
>>>>>>>>>>> Thanks!
>>>>>>>>>> I've pushed it back to garming-gpo-david-rebased after some
>>>>>>>>>> squashing.
>>>>>>>>>>
>>>>>>>>>> The diff is:
>>>>>>>>>>
>>>>>>>>>> diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
>>>>>>>>>> index 38844e5a43e..4f53981ec83 100644
>>>>>>>>>> --- a/libgpo/pygpo.c
>>>>>>>>>> +++ b/libgpo/pygpo.c
>>>>>>>>>> @@ -70,7 +70,7 @@ static PyObject *py_gpo_get_unix_path(PyObject
>>>>>>>>>> *self, PyObject *args,
>>>>>>>>>>            TALLOC_CTX *frame = NULL;
>>>>>>>>>>            static const char *kwlist[] = {"cache_dir", NULL};
>>>>>>>>>>            struct GROUP_POLICY_OBJECT *gpo_ptr \
>>>>>>>>>> -               = pytalloc_get_type(self, struct
>>>>>>>>>> GROUP_POLICY_OBJECT);
>>>>>>>>>> +               = (struct GROUP_POLICY_OBJECT
>>>>>>>>>> *)pytalloc_get_ptr(self);
>>>>>>>>>>              if (!PyArg_ParseTupleAndKeywords(args, kwds, "|s",
>>>>>>>>>> discard_const_p(char *, kwlist), &cache_dir)) {
>>>>>>>>>>                    PyErr_SetString(PyExc_SystemError, "Failed to
>>>>>>>>>> parse
>>>>>>>>>> arguments to gpo_get_unix_path()");
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I'll try to get this reviewed into master during the next little
>>>>>>>>>> while.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Andrew Bartlett
>>>>>>>>>>
>