Bug 7387 - s3: do not endless loop in process_registry_globals()
s3: do not endless loop in process_registry_globals()
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools
All Linux
: P3 normal
: ---
Assigned To: Michael Adam
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2010-04-23 10:39 UTC by Lars Müller
Modified: 2010-05-10 06:10 UTC (History)
0 users

See Also:

Patch applying to master. (1.19 KB, patch)
2010-04-23 10:41 UTC, Lars Müller
obnox: review-

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Müller 2010-04-23 10:39:29 UTC
In the case of "include = registry" process_registry_globals() caused an endless loop catched by the maximume include detection.

This was to trigger by:

a) Adding "include = registry" to the smb.conf global section
b) Calling "net conf import /etc/samba/smb.conf"
c) Calling "net conf drop"

The result was:
[2010/04/23 16:44:05.760955,  0] param/loadparm.c:7257(handle_include) Error: Maximum include depth (100) exceeded!
Comment 1 Lars Müller 2010-04-23 10:41:12 UTC
Created attachment 5650 [details]
Patch applying to master.
Comment 2 Lars Müller 2010-04-23 13:44:58 UTC
With 3.5.2 build without the patch we see:

maximegalon:~ # smbpasswd -a lmuelle
Can't load /etc/samba/smb.conf - run testparm to debug it

And as soon as I remove "include = registry" it works.

While launching smbd we see:

Apr 23 19:13:04 maximegalon smbd[11783]: [2010/04/23 19:13:04.576189,  0] param/loadparm.c:7257(handle_include)
Apr 23 19:13:04 maximegalon smbd[11783]:   Error: Maximum include depth (100) exceeded!

Using builds including the patch lead to

[2010/04/23 20:40:42,  0] smbd/server.c:1119(main)
  smbd version 3.5.2-3.1-2353-SUSE-SL11.2 started.

even if "include = registry" is part of the global section,
Comment 3 Michael Adam 2010-04-23 16:35:54 UTC
If I am not mistaken, your patch disables activation of the registry global section completely (process_registry_globals() does not process the registry
globals any more afterwards). I don't think this is desired.

Also this is not a mere problem of the registry configuration. There is
generally no depth in the stacking of inclusions in loadparm.c. You can construct a corresponding example with text files only:

include = smb.conf.2

include = smb.conf.2

This is basically the same situation as produced with your example, only with text files.

The problem in your example is the net conf import which also imports the include=registry setting, so this leads to the loop. IMHO you simply created an invalid configuration. But as shown above the corresponding situation can be produced using your text editor, so I would suggest to close this bug as invalid.

The only thing we could do is to decativate the include = registry special semantics when found in the includes of the global registry config section, but to me this seems the wrong thing to do, as it only catches one special case (not for instance the ping-pong case between a text file and the registry...). 
Right now we have a nice uniform treatment of config loops (i.e. invalid configs), no matter if registry or text config.
Comment 4 Lars Müller 2010-04-24 08:18:19 UTC
Intended result closing as invalid.