Winbindd problem enumerating lots of groups

[Ken Cross]

Sure, it isn't obvious.

"start" isn't where the list is in number of entries, it's an index into
some data structure (coming from the DC, I think).  The num_entries
would increment by 513 (in my tests), but cli_samr_enum_dom_groups would
set start to 7509 after the first call.  Basically,
cli_samr_enum_dom_groups must manage start once it's initialized to
zero.

The symptoms that I saw were large numbers of duplicate group names.  I
have ~2000 groups on this test server, but enum_groups returned > 9000.
It was because start only got incremented by 513 rather than 7509 each
time through the loop.

Another good data point is the fact that it was broken before I changed
it and worked after.  ;-)

Ken


-----Original Message-----
From: samba-technical-admin at lists.samba.org
[mailto:samba-technical-admin at lists.samba.org] On Behalf Of
jra at dp.samba.org
Sent: Sunday, January 05, 2003 11:03 PM
To: Ken Cross
Cc: samba-technical at samba.org
Subject: Re: Winbindd problem enumerating lots of groups


On Sun, Jan 05, 2003 at 07:22:33PM -0500, Ken Cross wrote:
> I found and fixed a problem where winbindd had trouble enumerating
> *lots* of groups (~2000 in my case).  This is in SAMBA_3_0, but it's 
> in other versions, too.
> 
> The problem was in the enum_dom_groups routine in winbindd_rcp.c, line
> 153:
> 
>   do {
>     struct acct_info *info2 = NULL;
>     uint32 count = 0, start = *num_entries;  <-- here
> 
> "start" is being re-initialized every time through the loop.  It 
> mustn't be.
> 
> The fix is easy - move it to the top of the routine a few lines up:
> 
>   uint32 start = 0;

Are you sure this is correct ? Yes, start is being initialized to
*num_entries (which starts at zero the first time through the
loop) but then at the bottom of the loop *num_entries is incremented as
so :

(*num_entries) += count;

So the next time around the loop start will be incremented
by count. It is then used as the next group index to fetch
in the call :

                status = cli_samr_enum_dom_groups(hnd->cli, mem_ctx2,
&dom_pol,
                                                  &start,
                                                  0xFFFF, /* buffer
size? */
                                                  &info2, &count);

The current code looks correct to me. Can you explain better why you
think this is wrong ?

Thanks,

	Jeremy.