very interesting!
I just noticed this in the libc manual. http://www.gnu.org/manual/glibc-2.2.5/html_node/Backtraces.html It could be pretty cool to have this built in to smb_panic(). -- Martin
Re: very interesting!
On 20 Mar 2003, Richard Sharpe [EMAIL PROTECTED] wrote: On Fri, 21 Mar 2003, Martin Pool wrote: I just noticed this in the libc manual. http://www.gnu.org/manual/glibc-2.2.5/html_node/Backtraces.html It could be pretty cool to have this built in to smb_panic(). But is it portable? It would of course have to be only used conditionally. But there's no reason why it couldn't be ifdefd. For the server I work on where gdb is not normally installed it would be highly useful. -- Martin
Re: 2.2.8 compile problem
On 17 Mar 2003, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote: checking whether to use included popt... ./popt checking configure summary... configure: error: summary failure. Aborting config Have a look in config.log. If you can't work out what's wrong from that, post the *relevant* sections to the [EMAIL PROTECTED] users list. -- Martin
overmalloc_safe_strcpy?
For developer mode, this seems to be the same as safe_strcpy: we clobber the specified region at runtime. Otherwise, it skips the static CHECK_STRING_SIZE call. I think this is meant to allow you to call it passing the address of an array whose size is less than the maxlength passed to safe_strcpy. CHECK_STRING_SIZE would normally trap on this because it expects either a string pointer, or an exact fit? Is that right? If so I'll add a comment to this effect -- and perhaps a plea not to use it in new code. -- Martin
(fwd) amigasamba?
Does anyone know about this? - Forwarded message from Larry Urquhart [EMAIL PROTECTED] - From: Larry Urquhart [EMAIL PROTECTED] Subject: amigasamba Date: Wed, 12 Mar 2003 21:28:49 -0800 To: [EMAIL PROTECTED] User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.2) Gecko/20030208 Netscape/7.02 X-Bogosity: No, tests=bogofilter, spamicity=0.024176, version=0.10.2 Hi. I don't know if this is the right place to ask this but I have been trying to visit the web site of www.amigasamba.org for over a week and it is off line. What's up ? I have questions to ask the administrator. Cheers Larry - End forwarded message - -- Martin
Re: tdb, valgrind, and mmap
On 13 Mar 2003, [EMAIL PROTECTED] wrote: 2 - Use IO not mmap when running under valgrind. Not so nice. thats why we have the 'use mmap = no' smb.conf option. It seems to work quite well and is fast enough for testing. OK, thanks. -- Martin
Re: [scottprive@earthlink.net: Re: Information on Samba QA process,Regression testsuites]
Please let's have this conversation on the list. I have Martin's notes.txt (Notes on Samba Testing Framework for Unittests). I think I misunderstood, and assumed there was a working (if immature) basic framework that hadn't been checked in yet. OK, we're at the planning stage? Some questions: We have some tiny tests but are basically still in planning. Tim and myself, and other people, are getting very annoyed with the lack of tests and we want to fix it soon. I'm going to commit our code into HEAD today or tomorrow. 1) For a supporting test framework, have you looked at STAF? I wish I had looked into STAF when I started my system tests. I ended up writing a whole lot of ssh/expect-driven functions to remotely reconfigure servers and clients, and of course installing/configuring Cygwin ssh on each NT box. This is the IBM suite? I think I've seen it before. I'll have a look at it. I feel a bit of NIH and would like a simple framework customized to our needs, but I'll try to look at STAF with an open mind. The above example is generating actions on clients, but STAF can also be used as an interface to maintain a resource pool. This was the first problem you were trying to solve, correct? 2) What value do you see in a distributed system suite, one that uses the same code path as clients... ex: a NT client using net use to attach a remote drivemap? Yes, I think both approaches are valuable. Driving NT clients is perhaps a bit harder: there needs to be some means to coordinate and drive them remotely. Testing via Samba client gives a somewhat less valid test, because there could be corresponding bugs in both our client and server. But it might be easier. I am wondering if system tests are redundant, but it's something I can code alone since I understand the requirements. For lower-level tests, I understand less and would wait for your first tests so I could study it, and then I'd need a suggested test to automate. 3) In the meantime of unit-planning, what else can do to help out? I already have a network environment I can work in here: one 2K PDC, an XP client, RH 8.0, plus on instance of VMWare on each box if I need older versions of Windows, etc. Learn Python :-) Write a draft test plan for some aspect of Samba. Point out problems that we'll hit in the future that we haven't thought of yet. ...? -- Martin
scalability of print_queue_update
I've been testing injection of many jobs (thousands) into a print queue, and am noticing that appliance_head samba seems to spend heaps of time in print_queue_update, trying to reconcile the output of lpq with samba's database. In particular, this is causing smbspool to give warnings because print_job_end is taking a long time to complete. (10s for tiny files.) One approach that was discussed a while ago is to have the lpd notify samba when jobs are completed, deleted, or changed. It could either give all the details sufficient to update the record, or (much simpler) just send an smbcontrol message to rescan the queue. Another approach, which I suppose is really just skirting the issue, would be to never run the update function while a client is waiting, but rather just at intervals when the timer expires. Possibly a separate smbd process might be forked to do this. Alternatively we might say that 40,000 jobs is a silly number to have queued. :-) I think we can also improve the efficiency of the current code without restructuring it; I'll send a patch for that shortly. Any thoughts on how this might be done better in 3.x? -- Martin
bug? non-default hash sizes in tdb
tdb (all branches) has the behaviour that when opening an existing database, if you don't specify the right hash size, the open will fail with EIO. This means for example that tdbtool can't open printing tdbs after jra's change to increase their has size to 5000. Wouldn't it be reasonable to ignore the hash_size parameter to tdb_open_ex when opening an existing tdb, and just use whatever is in the header? -- Martin
Re: scalability of print_queue_update
On 11 Mar 2003, [EMAIL PROTECTED] wrote: Doh ! Yeah, that's a really good idea. Wish I'd thought of it :-). Good, I'll send you a patch then. It would make the stress tests of a certain person in Roseville much harder to destroy Samba :-). I imagine her work as being similar to somebody testing new cars by driving from San Francisco to Boston with the park brake on. :-) :-) -- Martin
tdb, valgrind, and mmap
If you use tdbs under valgrind, and in particular if you run tdbtorture, you may get spurious uninitialized value warnings. I think this is because valgrind doesn't understand that the mmap'd area may be written to by other processes. Memory can, from the point of view of the grinded process, spontaneously become initialized. I can think of a few solutions. 1 - Write suppressions for Valgrind so that it doesn't complain about this. Probably the most reasonable but people need to remember to use them. 2 - Use IO not mmap when running under valgrind. Not so nice. 3 - Use the special valgrind macros to mark memory as valid at the right time. Probably too hard, and I'm not sure that any one process will ever really know which parts are valid. Any comments? -- Martin
valgrind_strlen?
Some files have a little valgrind_strlen function in there, I suppose to work around a Valgrind bug. Does anyone know what the bug was? There might be a cleaner solution. -- Martin
Re: Clean up winbindd locking
On 8 Mar 2003, Andrew Bartlett [EMAIL PROTECTED] wrote: On Sat, 2003-03-08 at 21:20, Tim Potter wrote: On Sat, Mar 08, 2003 at 09:10:23PM +1100, Andrew Bartlett wrote: Is there any reason I should not apply this patch to Samba HEAD? I think the patch was eaten by Mailman. Please re-send as text/plain. Without even seeing the patch (-: it's definitely a good idea. That looks very nice. -- Martin
Re: Information on Samba QA process, Regression testsuites
On 9 Mar 2003, Scott Prive [EMAIL PROTECTED] wrote:
Thanks Tim. So far, most of the tests I've seen in CVS are unit- or
small-tests, but as a software tester I'm more interested in integration
tests... which it sounds like you and Martin are working on.
Our framework is intended to eventually cover everything up to about
the system-test level, where you will run Samba tests against a number
of NT or Samba servers to check network interoperation.
A more-developed test suite using the same framework and design
principles is used in distcc:
http://cvs.samba.org/cgi-bin/cvsweb/distcc/test/
It is partially inspired by the very good Subversion test suite.
Any concrete practical suggestions you have on the design would be
welcome.
If it's less than 3MB, please email what you have.
(Sent by private mail)
This is in a private samba cvs directory; we'll probably make it
public in the future.
The design notes (not completely up-to-date) are attached.
--
Martin
Notes on Samba Testing Framework for Unittests
--
This is to be read after reading the notes.txt from comfychair. I'm
proposing a slightly more concrete description of what's described
there.
The model of having tests require named resources looks useful for
incorporation into a framework that can be run by many people in
widely different environments.
Some possible environments for running the test framework in are:
- Casual downloader of Samba compiling from source and just wants
to run 'make check'. May only have one Unix machine and a
handful of clients.
- Samba team member with access to a small number of other
machines or VMware sessions.
- PSA developer who may not have intimate knowledge of Samba
internals and is only interested in testing against the PSA.
- Non-team hacker wanting to run test suite after making small
hacks.
- Build farm environment (loaner machine with no physical access
or root privilege).
- HP BAT.
Developers in most of these environments are also potential test case
authors. It should be easy for people unfamiliar with the framework
to write new tests and have them work. We should provide examples and
the existing tests should well written and understandable.
Different types of tests:
- Tests that check Samba internals and link against
libbigballofmud.so. For example:
- Upper/lowercase string functions
- user_in_list() for large lists
- Tests that use the Samba Python extensions.
- Tests that execute Samba command line programs, for example
smbpasswd.
- Tests that require other resources on the network such as domain
controllers or PSAs.
- Tests that are performed on the documentation or the source code
such as:
- grep for common spelling mistakes made by abartlet (-:
- grep for company copyright (IBM, HP)
- Link to other existing testing frameworks (smbtorture,
abartlet's bash based build farm tests)
I propose a TestResourceManager which would be instantiated by a test
case. The test case would require(resourcename) as part of its
constructor and raise a comfychair.NotRun exception if the resource
was not present. A TestResource class could be defined which could
read a configuration file or examine a environment variable and
register a resource only if some condition was satisfied.
It would be nice to be able to completely separate the PSA testing
from the test framework. This would entail being able to define test
resources dynamically, possibly with a plugin type system.
class TestResourceManager:
def __init__(self, name):
self.resources = {}
def register(self, resource):
name = resource.name()
if self.resources.has_key(name):
raise Test manager already has resource %s % name
self.resources[name] = resource
def require(self, resource_name):
if not self.resources.has_key(resource_name):
raise Test manager does not have resources %s % resource_name
class TestResource:
def __init__(self, name):
self.name = name
def name(self):
return self.name
import os
trm = TestResourceManager()
if os.getuid() == 0:
trm.register(TestResource(root))
A config-o-matic Python module can take a list of machines and
administrator%password entries and classify them by operating system
version and service pack. These resources would be registered with
the TestResourceManager.
Some random thoughts about named resources for network servers:
require(nt4.sp3)
require(nt4.domaincontroller)
require(psa)
Some kind of format for location of passwords, libraries:
require(exec(smbpasswd))
require(lib(bigballofmud))
maybe require(exec.smbpasswd) looks nicer...
The require() function could return a dictionary of configuration
information or some handle to fetch dynamic information on.
[PATCH] draft: better string overflow checking (was: memorycorruption in SAMBA_3_0)
I was thinking about Andrew's fstring-overflow patch from a few weeks
ago: for developer builds, it touches the last byte of a string buffer
to check that it's as long as it should be.
This should be reasonably helpful in catching string overflows on the
heap, but not so good on the stack, because the program can probably
write arbitrarily far past stack variables without trapping, even
under Valgrind. Writing a \0 in there will damage *something* and
probably make the program crash, but it won't be very obvious. I
think this might have been what Jerry saw the other day.
I think this patch is better: it thoroughly clobbers the contents of
string buffers to make any fstring/pstring/dynamic confusion obvious.
Here is an example that is caught in developer builds with this patch,
but is hard to catch otherwise:
#include includes.h
int main(void)
{
fstring dest;
pstrcpy(dest, hello);
return 0;
}
This fails with an obvious message under gdb:
#0 0xf1f1f1f1 in ?? ()
Cannot access memory at address 0xf1f1f1f1
Please don't apply this yet because I want to see if it catches any
bugs, but I'd love to hear comments.
--- util_str.c.~1.83.~ 2003-03-05 15:24:08.0 +1100
+++ util_str.c 2003-03-05 15:57:16.0 +1100
@@ -430,6 +430,27 @@ BOOL str_is_all(const char *s,char c)
return True;
}
+
+/**
+ * In developer builds, clobber a region of memory.
+ *
+ * If we think a string buffer is longer than it really is, this ought
+ * to make the failure obvious, by segfaulting (if in the heap) or by
+ * killing the return address (on the stack), or by trapping under a
+ * memory debugger.
+ *
+ * This is meant to catch possible string overflows, even if the
+ * actual string copied is not big enough to cause an overflow.
+ **/
+void clobber_region(char *dest, size_t len)
+{
+#ifdef DEVELOPER
+ /* F1 is odd and 0xf1f1f1f1 shouldn't be a valid pointer */
+ memset(dest, 0xF1, len);
+#endif
+}
+
+
/**
Safe string copy into a known length string. maxlength does not
include the terminating zero.
@@ -444,13 +465,7 @@ char *safe_strcpy(char *dest,const char
return NULL;
}
-#ifdef DEVELOPER
- /* We intentionally write out at the extremity of the destination
-* string. If the destination is too short (e.g. pstrcpy into mallocd
-* or fstring) then this should cause an error under a memory
-* checker. */
- dest[maxlength] = '\0';
-#endif
+ clobber_region(dest, maxlength+1);
if (!src) {
*dest = 0;
@@ -499,6 +514,8 @@ char *safe_strcat(char *dest, const char
dest[maxlength] = 0;
return NULL;
}
+
+ clobber_region(dest + dest_len, maxlength - src_len);
memcpy(dest[dest_len], src, src_len);
dest[dest_len + src_len] = 0;
@@ -516,6 +533,8 @@ char *alpha_strcpy(char *dest, const cha
{
size_t len, i;
+ clobber_region(dest, maxlength);
+
if (!dest) {
DEBUG(0,(ERROR: NULL dest in alpha_strcpy\n));
return NULL;
@@ -554,8 +573,12 @@ char *alpha_strcpy(char *dest, const cha
char *StrnCpy(char *dest,const char *src,size_t n)
{
char *d = dest;
+
+ clobber_region(dest, n+1);
+
if (!dest)
return(NULL);
+
if (!src) {
*dest = 0;
return(dest);
@@ -576,6 +599,8 @@ char *strncpyn(char *dest, const char *s
char *p;
size_t str_len;
+ clobber_region(dest, n+1);
+
p = strchr_m(src, c);
if (p == NULL) {
DEBUG(5, (strncpyn: separator character (%c) not found\n, c));
--
Martin
Re: win2000 server, linux client
On 21 Feb 2003, Marco Eyzaguirre [EMAIL PROTECTED] wrote: I have a win2000 domain controller in subnet 172.17.0.x and a linux server in 172.17.20.x (DHCP assign) The DC not solve the ip / netbiosname relation (A record i think..) like a win98 / NT /2000 If i ping the linux server from outside 172.17.20.x i receive this : ping 172.17.20.1 -- response ping dgalinux -- time out (dgalinux is the netbiosname of the linux server) Is this a problem of samba? i was searching a solution, patch or something like that, but i found nothing.. i have Redhat linux 8.0 and Samba 2.2.5 No, this is probably a routing or networking problem, not a Samba problem. Please try asking in a Linux support group -- you can probably find a Spanish language one. -- Martin
Re: technical support
On 19 Feb 2003, HAKIZIMANA Claude [EMAIL PROTECTED] wrote: Can you help me? You need to ask this kind of question on the users list, [EMAIL PROTECTED] -- Martin
[PATCH] leak in init_valid_table
util_unistr.c/init_valid_table in HEAD and 3.0 causes a 64k leak every
time the configuration is loaded if there is no valid.dat file
installed. (The pointer to the malloc'd valid_table is clobbered by
the call to map_file.)
It looks like it is intended that the valid table be recreated on each
reload in case the dos codepage has changed. Is that right?
The commit comment for 1.87 says that this should remove the need for
valid.dat. Rather than fixing the leak I wonder if it would be better
to now just remove valid.dat support altogether?
If you don't want to do that, I think this patch will remove the leak.
--- util_unistr.c.~1.99.~ 2003-02-21 14:05:33.0 +1100
+++ util_unistr.c 2003-02-21 14:14:20.0 +1100
@@ -105,27 +105,34 @@ static int check_dos_char(smb_ucs2_t c)
**/
void init_valid_table(void)
{
- static int initialised;
static int mapped_file;
int i;
const char *allowed = .!#$%'()_-@^`~;
+ uint8 *valid_file;
- if (initialised mapped_file) return;
- initialised = 1;
+ if (mapped_file) {
+ /* Can't unmap files, so stick with what we have */
+ return;
+ }
- valid_table = map_file(lib_path(valid.dat), 0x1);
- if (valid_table) {
+ valid_file = map_file(lib_path(valid.dat), 0x1);
+ if (valid_file) {
+ valid_table = valid_file;
mapped_file = 1;
return;
}
- /* Otherwise, using a dynamically loaded one. */
+ /* Otherwise, we're using a dynamically created valid_table.
+* It might need to be regenerated if the code page changed.
+* We know that we're not using a mapped file, so we can
+* free() the old one. */
if (valid_table) free(valid_table);
DEBUG(2,(creating default valid table\n));
valid_table = malloc(0x1);
- for (i=0;i128;i++) valid_table[i] = isalnum(i) ||
- strchr(allowed,i);
+ for (i=0;i128;i++)
+ valid_table[i] = isalnum(i) || strchr(allowed,i);
+
for (;i0x1;i++) {
smb_ucs2_t c;
SSVAL(c, 0, i);
--
Martin
Re: SEGFAULT in HEAD
On 20 Feb 2003, Stefan (metze) Metzmacher [EMAIL PROTECTED] wrote: and the backtrace: #0 0x40325079 in wait4 () from /lib/libc.so.6 #1 0x4039d944 in __DTOR_END__ () from /lib/libc.so.6 #2 0x402c80e6 in system () from /lib/libc.so.6 #3 0x08183e8c in smb_panic (why=0x8221974 internal error) at lib/util.c:1406 #4 0x08174423 in fault_report (sig=11) at lib/fault.c:41 #5 0x08174476 in sig_fault (sig=11) at lib/fault.c:61 #6 signal handler called #7 0x402f1990 in free () from /lib/libc.so.6 (I'm probably telling you something you already know, but anyhow...) The fault occurred here. A segv in free() typically indicates that the heap has been corrupted at some earlier point, perhaps by writing over a dynamic buffer, a double-free() or something similar. A less common cause is passing a bad pointer to free(). The best way to debug this is to recreate the problem under Valgrind, or something like dmalloc or Purify. #8 0x080a3340 in reply_trans2 (conn=0x8312828, inbuf=0x404a7008 , outbuf=0x404c8008 , length=76, bufsize=131072) at smbd/trans2.c:3318 #9 0x080b442f in switch_message (type=50, inbuf=0x404a7008 , outbuf=0x404c8008 , size=76, bufsize=131072) at smbd/process.c:758 #10 0x080b44d0 in construct_reply (inbuf=0x404a7008 , outbuf=0x404c8008 , size=76, bufsize=131072) at smbd/process.c:788 #11 0x080b478e in process_smb (inbuf=0x404a7008 , outbuf=0x404c8008 ) at smbd/process.c:889 #12 0x080b5058 in smbd_process () at smbd/process.c:1291 #13 0x0807546f in main (argc=2, argv=0xb824) at smbd/server.c:904 #14 0x4029a7ee in __libc_start_main () from /lib/libc.so.6 -- Martin msg06230/pgp0.pgp Description: PGP signature
3.0 commit policy?
What is the commit policy for 3.0? Should all changes be moved from HEAD except ones that are explicitly too risky? Or only bug fixes? What about documentation? -- Martin
source location for samba test harnesses
After talking to Tim, I wanted to start committing to HEAD some test harnesses that will exercise internal bits of Samba, such as StrCaseCmp to start with. These will be small C files that link to -lbigballofmud and allow particular functions to be exercised from the command line, with a view to eventually calling them from a Python testing framework. They compile to relatively small executables, and they won't be built by default. What is a good location for the source? torture/ seems to be the closest existing directory. -- Martin
Re: Annoying Minor Bug In Winbind 2.2.x
On 18 Feb 2003, Boyce, Nick [EMAIL PROTECTED] wrote: I'm sorry - I'm probably doing something dumb, but I still get failures even with this patch - first, if I save the patch as it appeared in my Outlook window, then line 25 consists of a single left brace char, which results in : You can also download the patch from here http://pserver.samba.org/cgi-bin/cvsweb/samba/source/lib/util_sock.c.diff?r1=1.16.4.36r2=1.16.4.37 In general you can try using view source to get a version that's not folded/spindled/mutilated by Outlook, or the very cool unwrapdiff to try to fix the line wrapping. Thanks for persisting. -- Martin
Re: Annoying Minor Bug In Winbind 2.2.x
On 17 Feb 2003, Boyce, Nick [EMAIL PROTECTED] wrote: OK - I've been trying to apply the patch that Tim posted (to supersede Martin's first cut) to the Samba 2.2.7a source file for util_sock.c, but get errors applying the patch no matter what I do. Thanks for trying that. I guess the posted patch was against CVS, so could someone please repost the patch for the 2.2.7a-rel version of the file ? I'll do that. -- Martin If it's worth doing, it's worth doing for money.
Re: Annoying Minor Bug In Winbind 2.2.x
On 17 Feb 2003, Boyce, Nick [EMAIL PROTECTED] wrote: Here's what I get if I apply the posted patch : As I said I'll send you an update just for 2.2. But in general, in case you're interested, here are some tips on applying mismatched patches: MYBOX:/usr/local/src/samba-2.2.7a/source/lib# patch util_sock.c patch-util_sock.txt.orig patching file util_sock.c Hunk #1 succeeded at 1018 with fuzz 2 (offset 133 lines). Hunk #2 FAILED at 1037. Hunk #3 FAILED at 1094. 2 out of 3 hunks FAILED -- saving rejects to file util_sock.c.rej In general the best thing to do now is leave the main diff alone, and only work on the rejected parts in the .rej file. Basically you need to work out why patch thinks the 2.2 source file doesn't look like the before version of the rejected patch. After deleting the line containing #ifdef HAVE_UNIXSOCKET (because I noticed it doesn't appear in my 2.2.7a version of util_sock.c), I get a little further : MYBOX:/usr/local/src/samba-2.2.7a/source/lib# patch util_sock.c patch-util_sock.txt patching file util_sock.c Hunk #1 succeeded at 1018 with fuzz 2 (offset 133 lines). patch: malformed patch at line 102: @@ -966,25 +961,26 @@ Do you mean you deleted that line from the patch? That's probably what is causing the malformed patch error: the line numbers in the patch no longer add up. However, if you install the patchutils package, then you can run recountdiff to fix the lines up after you edit a diff, and it should then apply. patchutils is very very cool. -- Martin
interesting fact about StrCaseCmp
StrCaseCmp (and strequal) in HEAD has the interesting side-effect that it only compares the first PSTRING_LEN (1024) bytes of the strings. I suppose comparing strings longer than that is probably a bit unlikely, but it still seems kind of dangerous. Would it be OK to change it to use dynamic allocation? -- Martin
Re: interesting fact about StrCaseCmp
On 18 Feb 2003, Andrew Bartlett [EMAIL PROTECTED] wrote:
Possibly only for long strings? But then that is probably
micro-optimization.
If we really cared about optimizing this function, then we would
compare character-by-character rather than converting both strings to
uppercase first. This is a bit hard for some wierd encodings I know,
but it ought to be possible to do it in charcnv.c.
The case where we compare, for example, a thousand-character string to
the empty string is ridiculously slow at the moment.
I don't know if this is a problem for Samba overall or not, so I'm not
touching it at the moment.
int StrCaseCmp(const char *s, const char *t)
{
pstring buf1, buf2;
unix_strupper(s, strlen(s)+1, buf1, sizeof(buf1));
unix_strupper(t, strlen(t)+1, buf2, sizeof(buf2));
return strcmp(buf1,buf2);
}
--
Martin
Re: Detecting true64
On 17 Feb 2003, [EMAIL PROTECTED] wrote: Does anyone know how to detect a truu64 system in configure.in ? I'm going through my patchlist and there is a big optimisation that can be done on systems where the getgrnam() call works (True64 is listed as the only broken system) and I'd like to add this to all branches by adding a BROKEN_GETGRNAM define for True64. Note that it really is Tru64. I guess misspelling makes it more dynamic. config.guess should detect a hostname of something like alphaev7-dec-osf4.0f See e.g. http://studio.imagemagick.org/pipermail/magick-bugs/2002-November/000881.html configure.in should call AC_CANONICAL_HOST, and then examine $host_os, probably with something like case $host_os in osf*) ;; The config.guess is actually quite old compared to the current GNU distribution, so I might update that in HEAD and 3_0. This might improve correctness or robustness in detecting some platforms. -- Martin We use Film Gimp on all talking animal jobs -- Caroline Dahllöf
Doxygen janitor?
Is there any kind of consensus (he says, hopefully) that Doxygen is a good idea? If I'm looking at code is it OK to cleanup comments into standard form? -- Martin
Re: interesting fact about StrCaseCmp
On 18 Feb 2003, [EMAIL PROTECTED] wrote: What exactly do you want to do here ? I'm not clear what you mean? The thing I noticed is that StrCaseCmp (and indeed many charcnv function) truncate strings to 1024 characters. I got here following a Valgrind assertion which may or may not be related, but it certainly seems like a bug. What I was proposing in the first instance was to use talloced or malloced buffers rather than a fixed 1024 byte space. There are already some charcnv routines that do this and in any case it is straightforward to do it for other cases using the standard measure-allocate-copy method. Andrew expressed concern that allocating buffers would be inefficient, but these functions are already extremely inefficient so I don't think an extra malloc would matter. In fact, it's worse, because unix_strupper (for example) uses a 1024-*byte* buffer to hold a UCS2 string. I don't think it will overflow, but it will truncate any strings that pass through down to 512 characters. It's not so hard to imagine a 512-character string. Basically I just wanted to push on with moving away from pstrings/fstrings, which I understood from Andrew Tim to be the current direction. -- Martin
[PATCH] string_to_sid audit
In several cases, the return code from string_to_sid is not checked.
So if the user enters a syntactically invalid SID, the program will
proceed to use uninitialized data.
This patch checks for a few such cases that I found. Can somebody
please review it?
Index: groupdb/mapping.c
===
RCS file: /data/cvs/samba/source/groupdb/mapping.c,v
retrieving revision 1.44
diff -u -U6 -r1.44 mapping.c
--- groupdb/mapping.c 2 Jan 2003 09:07:02 - 1.44
+++ groupdb/mapping.c 17 Feb 2003 04:21:37 -
@@ -301,13 +301,17 @@
if(!init_group_mapping()) {
DEBUG(0,(failed to initialize group mapping));
return(False);
}
map.gid=gid;
- string_to_sid(map.sid, sid);
+ if (!string_to_sid(map.sid, sid)) {
+ DEBUG(0, (string_to_sid failed: %s, sid));
+ return False;
+ }
+
map.sid_name_use=sid_name_use;
fstrcpy(map.nt_name, nt_name);
fstrcpy(map.comment, comment);
map.systemaccount=systemaccount;
map.priv_set.count=priv_set.count;
Index: nsswitch/wb_client.c
===
RCS file: /data/cvs/samba/source/nsswitch/wb_client.c,v
retrieving revision 1.40
diff -u -U6 -r1.40 wb_client.c
--- nsswitch/wb_client.c7 Aug 2002 07:28:24 - 1.40
+++ nsswitch/wb_client.c17 Feb 2003 04:21:37 -
@@ -53,13 +53,14 @@
fstrcpy(request.data.name.dom_name, dom_name);
fstrcpy(request.data.name.name, name);
if ((result = winbindd_request(WINBINDD_LOOKUPNAME, request,
response)) == NSS_STATUS_SUCCESS) {
- string_to_sid(sid, response.data.sid.sid);
+ if (!string_to_sid(sid, response.data.sid.sid))
+ return False;
*name_type = (enum SID_NAME_USE)response.data.sid.type;
}
return result == NSS_STATUS_SUCCESS;
}
@@ -155,13 +156,14 @@
result = winbindd_request(WINBINDD_UID_TO_SID, request, response);
/* Copy out result */
if (result == NSS_STATUS_SUCCESS) {
- string_to_sid(sid, response.data.sid.sid);
+ if (!string_to_sid(sid, response.data.sid.sid))
+ return False;
} else {
sid_copy(sid, global_sid_NULL);
}
return (result == NSS_STATUS_SUCCESS);
}
@@ -221,13 +223,14 @@
result = winbindd_request(WINBINDD_GID_TO_SID, request, response);
/* Copy out result */
if (result == NSS_STATUS_SUCCESS) {
- string_to_sid(sid, response.data.sid.sid);
+ if (!string_to_sid(sid, response.data.sid.sid))
+ return False;
} else {
sid_copy(sid, global_sid_NULL);
}
return (result == NSS_STATUS_SUCCESS);
}
Index: python/py_lsa.c
===
RCS file: /data/cvs/samba/source/python/py_lsa.c,v
retrieving revision 1.16
diff -u -U6 -r1.16 py_lsa.c
--- python/py_lsa.c 23 Dec 2002 23:53:55 - 1.16
+++ python/py_lsa.c 17 Feb 2003 04:21:37 -
@@ -232,23 +232,29 @@
memset(sids, 0, num_sids * sizeof(DOM_SID));
for (i = 0; i num_sids; i++) {
PyObject *obj = PyList_GetItem(py_sids, i);
- string_to_sid(sids[i], PyString_AsString(obj));
+ if (!string_to_sid(sids[i], PyString_AsString(obj))) {
+ PyErr_SetString(PyExc_ValueError, string_to_sid
+failed);
+ return NULL;
+ }
}
} else {
/* Just a single element */
num_sids = 1;
sids = (DOM_SID *)talloc(hnd-mem_ctx, sizeof(DOM_SID));
- string_to_sid(sids[0], PyString_AsString(py_sids));
+ if (!string_to_sid(sids[0], PyString_AsString(py_sids))) {
+ PyErr_SetString(PyExc_ValueError, string_to_sid failed);
+ return NULL;
+ }
}
ntstatus = cli_lsa_lookup_sids(hnd-cli, hnd-mem_ctx, hnd-pol,
num_sids, sids, domains, names,
types);
Index: rpc_parse/parse_net.c
===
RCS file: /data/cvs/samba/source/rpc_parse/parse_net.c,v
retrieving revision 1.102
diff -u -U6 -r1.102 parse_net.c
--- rpc_parse/parse_net.c 14 Feb 2003 23:04:02 - 1.102
+++ rpc_parse/parse_net.c 17 Feb 2003 04:21:38 -
@@ -882,17 +882,19 @@
*ppsids = (DOM_SID2 *)talloc_zero(ctx, count * sizeof(DOM_SID2));
Re: query about rpcclient process_cmd:
On 13 Feb 2003, Gerald (Jerry) Carter [EMAIL PROTECTED] wrote:
Hash: SHA1
On Thu, 13 Feb 2003, Martin Pool wrote:
rpcclient.c/process_cmd has
if (cmd[strlen(cmd) - 1] == '\n')
cmd[strlen(cmd) - 1] = '\0';
if (!next_token(p, buf, , sizeof(buf))) {
return NT_STATUS_OK;
}
/* strip the trainly \n if it exsists */
len = strlen(buf);
if (buf[len-1] == '\n')
buf[len-1] = '\0';
Isn't the second check for newline redundant?
Looks like it to me.
Actually, Tim made the very good suggestion that we ought to change
this to use popt, rather than the current half-assed string handling.
I might make a patch to do this later.
--
Martin
msg06064/pgp0.pgp
Description: PGP signature
Re: password quality script aka --with-cracklib replacement
On 14 Feb 2003, Andrew Bartlett [EMAIL PROTECTED] wrote: a) If we want the password-quality script to handle this, I think we'll all agree, storing clear text password is really not a good idea. Perhaps the interface should provide the new encrypted passwords to the external program -- if administrators don't have easy access to the encrypted password, some will probably endup use the cleartext one :( I think this complicates the issue - smbd would then be involved in key management etc - and that just gets messy. A generally useful interface needs to provide the cleartext password. (For example, as I mentioned earlier, our domain controller forbids passwords which have too many characters in common with an earlier password. I can't see this in the Domain Controller Security Policy, but perhaps it's a plug in or configured somewhere else.) Hopefully we can ship enough standard modules that people won't want to write random insecure implementations. But if they want to, they can. We already use PAM, and we should use it for all the things it's intended to do, but this is not something it was intended to do. I really disagree, I think this is exactly the sort of thing PAM is good for: providing a standard interface for doing authentication-related things, without needing to invent a new interface for each. PAM replaces a pile of little messy solutions and we should really not add a new custom protocol back in. Using PAM also allows the code to be reused for passwords changed through other mechanisms (web interfaces, ...) (Because we are not asking it to store the password). I don't understand this one... Having PAM modules which check a proposed password change rather than storing it is absolutely fine -- look at the existing strength-checking systems. http://www.kernel.org/pub/linux/libs/pam/modules.html Basically PAM just provides a standard way of having a little conversation about a new password. Ain't people moving towards pdb LDAP backend using the included LDAP support in Samba instead of using PAM? LDAP and PAM solve different problems. PAM is the interface from the application to the security module; LDAP is the database. It could make sense to have a module which talks PAM to Samba and stores its data in LDAP. We could add an attribute password quality runas root (true/false) in smb.conf . Should we? Only add it if you can describe a case where running it as root wouldn't work. 5) Protocol Do we agree on the following for version: 1\n? I think you should use a PAM API rather than a subprocess. But if you insist on a subprocess, I think this is OK. The script would only send a different value if it was found that MS was using a different value. Allowing a hexadecimal number is both perfectly practical, and solves this little issue. Yes, please just send the number. -- Martin
Re: password quality script aka --with-cracklib replacement
On 14 Feb 2003, Andrew Bartlett [EMAIL PROTECTED] wrote: Do we even need to save the decrypted password? A colleague once saved old encrypted passwords to allow the do they really know the old one test to be done via challange-response. Different scripts might want to store various things derived from the old password. If you want to prevent reuse of old passwords, then you should store the result of some kind of trapdoor function applied to the function. Hashing the password is desirable because it helps avoid compromise of old passwords if an attacker accesses the database. However, some people want to check that the new password is not similar to a previous password, using some kind of soundex or edit-distance algorithm. I'm pretty sure NT domain controllers can do this, or be programmed to do so. This requires storing the unencrypted old passwords so that you can do such comparisons. (Or I suppose you could be really smart and invent a hashed format such that similar passwords produce similar hashes, without allowing the password to be retrieved from the hash.) But in general I think you want to allow the script the chance of having the unencrypted, unhashed, password. If it wants to discard information then it can. -- Martin msg06067/pgp0.pgp Description: PGP signature
Re: password quality script aka --with-cracklib replacement
On 12 Feb 2003, Andrew Bartlett [EMAIL PROTECTED] wrote: Because we don't have the old password, doing this via PAM doesn't work. The pam_cracklib module doesn't apply the test if it's run as root, and won't run without the old password as a normal user. I know it won't work with the existing pam_cracklib module. What I was asking was whether it is possible to write a new module that connects using PAM and which does provide the right checks, rather than inventing a new plugin interface. The PAM module might store previous passwords in a database (e.g. tdb) that it maintains. Every time a password is set, it gets put in there, with any other appropriate information (date?). When a new password-setting attempt is made, it checks against the history, plus other strength checks. I know PAM's configuration method is a bit gross, but standard is better than better. Since libraries can't be setuid it would need to be invoked by smbd as root, but that probably make sense anyhow as you say. Personally I would use something like a tdbpacked string, which avoids worries about strange characters or string parsing, and is easy to handle in C, Perl, and Python. This is an interesting idea - but how available is the interface for our particular custom string format? There is a Python library to decode them. Writing one for Perl would be quite trivial: perhaps 100 lines and half an hour. NT_STATUS_OK # New password accepted NT_STATUS_ACCESS_DENIED # Error occured in the script NT_STATUS_PASSWORD_RESTRICTION # Too short, weak, etc. I suggested the string - I don't think sending the hex value adds much, and makes it less self-documenting. Parsing the string is trivial, as we already have the lookup routines (I use it for a custom-hack auth module). We could certainly allow both - which would allow a new NTSTATUS code to be used in the unlikely event a useful one appears in this context. You can perhaps imagine a script that wants to be a proxy to some other service that returns arbitrary error codes, so translating to and from strings would be a waste of time and effort. So we ought to at least allow numeric values, as you say. Why make it optional, though? There's already space for a human-readable description string. Presumably the script will use symbolic names for all the values, so the code will be self-documenting which is the most important part. All these protocols send numeric values, so it's better to be consistent. (For example, wbinfo prints out hex ntstatus values.) -- Martin
samba doxygen autogeneration
I've set up a cron script to autogenerate Doxygen from HEAD, 2_2 and APPLIANCE_HEAD on samba.org: http://samba.org/doxygen/ This rebuilds every day from the anoncvs checkout. Thanks to Andrew for reminding me to do this. -- Martin
Re: Winbindd limited by select
On 12 Feb 2003, Michael Steffens [EMAIL PROTECTED] wrote: It's 60 by default after installation, but is tunable (with reboot). Maybe hp should sell per-fd licences :-/ The solution (and this should also work on other platforms) was to have winbindd housekeep its client connections by shutting down idle connections, and have clients reconnect when required: http://lists.samba.org/pipermail/samba-technical/2003-February/042210.html The threshold was chosen to be 100 active connections, which keeps winbindd well below 300 FDs. Below 140, actually, including network sockets and open database and log files. I wonder how well it would work to have one winbindd process per n clients, in addition to shutting down idle clients... -- Martin msg06032/pgp0.pgp Description: PGP signature
query about rpcclient process_cmd:
rpcclient.c/process_cmd has
if (cmd[strlen(cmd) - 1] == '\n')
cmd[strlen(cmd) - 1] = '\0';
if (!next_token(p, buf, , sizeof(buf))) {
return NT_STATUS_OK;
}
/* strip the trainly \n if it exsists */
len = strlen(buf);
if (buf[len-1] == '\n')
buf[len-1] = '\0';
Isn't the second check for newline redundant?
--
Martin
Re: CVS update: samba/source/printing
On 12 Feb 2003, [EMAIL PROTECTED] wrote: $ cvs log -r1.9.2.14 tdb/tdbutil.c revision 1.9.2.14 date: 2002/11/27 01:51:43; author: jra; state: Exp; lines: +21 -25 SMBencrypt needs dos codepage also. Change tdb_pack/unpack to take a function pointer applied to all strings if it exists. Jeremy. Sorry about that. I'll watch for these nasty differences more closely in future. The patch I originally sent for this kept them in line -- it would have been better to do the conversion in the HP Python code than to put this in tdbpack. I'm sure you had a good reason in the heat of battle though. -- Martin
Re: rpcclient adddriver: core dump
On 12 Feb 2003, Ronan Waide [EMAIL PROTECTED] wrote: Samba HEAD Looks like it's triggered by not closing quotes: [root@workst1 root]# rpcclient -U admin%passwd -W GROUP workst1 -d2 added interface ip=192.168.168.250 bcast=192.168.168.255 nmask=255.255.255.0 rpcclient $ adddriver Windows 4.0 HP CL 8500 - PCL:HPCPCLA.DLL:HP_LJ85.PPD:HPCPCLA1.DLL:H Segmentation fault i'll have a look. (the second param to addriver is incomplete due to a cut-and-paste mishap; hitting return on it produces the segv.) -- Martin
Re: password quality script aka --with-cracklib replacement
On 11 Feb 2003, Pierre Belanger [EMAIL PROTECTED] wrote: What is it? I have my own comments at the end ... From the documentation I wrote (even if I'm French I think it's not that bad!?!?!?): This looks good to me. Would it be possible to do this as a PAM module called by Samba? (Possibly not, or not appropriate, but I thought I'd ask.) Presumably if the script is set in the smb.conf but can't be run then Samba ought to fail closed and deny the change request. Is there any need to allow changes by the administrator to bypass this check? Full path to the script that will be called when a request for change password is received. It will be run by smbd as non-ROOT. The script is responsible to accept or refuse the new password based on its own rules. As an example, it can be a script refusing a weak password based on a dic- tionary word. OK, that sounds good. For things like preventing password reuse the script will need to keep a database of (probably hashed) old passwords. It's probably best to keep this functionality out of smbd. Are there permissions problems in the script maintaining state such as this? The interface ought to define whether the script will be run under the uid of the person whose password is being changed. Perhaps some scripts will need to be setgid to store information in a file not accessible to the user -- after all the point of this is to impose restrictions on what the user can do. In fact, it seems to me that to be really secure, this operation *must not* be done under the uid whose password is being changed: enforcing security from a process under the control of that user is very weak. Samba back end communicates with the password quality program by writing data to its STDIN and reading data from its STDOUT. Samba writes a block of data in Field:value\n terminated by a .\n at the beginning of a new line. 1) smbd to --- Password quality script STDIN Version:smbd-version-string\n Username:username\n Fullname:fullname\n Password:new-password\n .\n This seems conceptually good, but I have a few comments on the format. The interface definition ought to say that the strings are sent in UTF-8. Using \n for a delimiter seems like asking for trouble; such as a fullname or new password containing that character. I'd suggest \0. I don't think this has to be a human-readable protocol. I am not sure that the headings like Version: really add much. If you're going to use something like RFC822 then it ought to be a closer match, with a space after the colon. Similarly, rather than the final dot, why not close the connection? Perhaps it would be better to specify the protocol version, rather than the smbd version? i.e. just send Version: 1 at first. A script written today doesn't know what future version numbers will be. Personally I would use something like a tdbpacked string, which avoids worries about strange characters or string parsing, and is easy to handle in C, Perl, and Python. NTStatus:ntstatus-string\n Result:result-string\n .\n The ntstatus-string value must used one the pre- defined NTSTATUS (nterr.h) values. In this context: NT_STATUS_OK # New password accepted NT_STATUS_ACCESS_DENIED # Error occured in the script NT_STATUS_PASSWORD_RESTRICTION # Too short, weak, etc. I think you should send the hex value, not the string. The result-string value is used to provide informa- tion (debug info) to smbd. For examples: NTStatus:NT_STATUS_PASSWORD_RESTRICTION\n Result:Password is based on dictionary word\n or NTStatus:NT_STATUS_OK\n Result:New password is accepted\n smbd will always return an error to the client and does not change the current password unless the NTStatus value is equal to NT_STATUS_OK and the exit status of the script equal to 0. Default: password quality script = empty string Example: password quality script = /usr/local/samba/sbin/password-quality.pl Do you wonder why sending the smbd-version-string? Perhaps in the future a new NTSTATUS will be added. The only way for the external program to know if it can use the new value is by knowing the version of Samba! There are other possible examples. If you send the hex value, then it doesn't matter if Samba understands the code or not -- it can just pass it back to the client. Anyhow, I don't think it's likely that the script authors will know
background updates of print queues via a dedicated process
The Samba 3.0 roadmap mentions this as a wishlist item for 3.x. I'm interested in looking at it. Has anybody else already worked on it? It seemed like it would involve a separate smbd process repeatedly parsing the output of lpq and feeding it into a database, rather than this being done on-demand from a regular smbd child. I suppose when some change is noticed it will have to send a message to prod other smbds. It might be good for there to also be a way for the spooler to notify Samba when something has happened, so as to avoid polling. This seems like it might help print server efficiency with many jobs or printers. Am I on the right track? -- Martin msg05981/pgp0.pgp Description: PGP signature
Re: background updates of print queues via a dedicated process
On 11 Feb 2003, Gerald (Jerry) Carter [EMAIL PROTECTED] wrote: It seemed like it would involve a separate smbd process repeatedly parsing the output of lpq and feeding it into a database, rather than this being done on-demand from a regular smbd child. I suppose when some change is noticed it will have to send a message to prod other smbds. or the child smbd can always assume that it is up to date (of course locking during an update makes sure that a smbd doesn't get a half done queue listing). I was thinking of the way smbd needs to notify waiting clients when the print queue changes. I guess the notification doesn't need to happen straight away. It might be good for there to also be a way for the spooler to notify Samba when something has happened, so as to avoid polling. CUPS might support this. I dunno. or it could be added to lpd of course. Yes, this might be added to the hp lpd, for example. Generating and parsing 50,000 lines of lpq output has got to be less efficient than lpq just sending job 585 complete as appropriate. This is kind of a second-stage task though. -- Martin msg05984/pgp0.pgp Description: PGP signature
Re: background updates of print queues via a dedicated process
On 12 Feb 2003, Tim Potter [EMAIL PROTECTED] wrote: My idea which I've probably told a couple of you is to use kernel dnotify stuff to work out when jobs are spooled or removed. So a daemon would get a signal when a spool file is created and add that to printing.tdb. When the file completes spooling lpd deletes it and the daemon gets another signal saying that file has deleted. A problem with that is that it exposes the internals of the lpd to samba, whereas at the moment they're more or less hidden. (Consider a spooler which has nested queue directories, like squid or Postfix.) But I'll leave space to add this mechanism as well. -- Martin
Re: Annoying Minor Bug In Winbind 2.2.x
On 7 Feb 2003, Boyce, Nick [EMAIL PROTECTED] wrote: Thanks - that was it. I now have a script /usr/local/bin/winbind, which does umask 000 /etc/init.d/winbind $1 umask 027 and everything is working ok now - I can stop restart winbind to my heart's content without any problem (well no socket directory permissions problems anyway ;-) You would be better off -- and you would be helping us too -- if you would apply the patch and let us know if it works. (I'm pretty sure it will, but it's always worth testing.) That way, rather than a temporary workaround, there will be a proper fix for future releases. Thanks, [ I'm afraid I always run with umask=027 ... it's a hangover from my mainframe days ... I can't get away from the idea that you should grant only the access that is needed ... all files world-readable by default ? ... Just Say No ] That's fine, winbindd ought to work with that. -- Martin
[PATCH] umask audit
Following on from the bug in winbindd this morning I did a quick grep
for umask.
In HEAD/client/client.c main(), there is a pair of calls to umask. It
looks to me like they're trying to retrieve the current umask without
changing it. However, the retrieved value is never used. (Did I miss
something?) Therefore I think the calls ought to be deleted.
--- client.c.~1.230.~ 2003-01-13 15:46:34.0 +1100
+++ client.c2003-02-07 15:33:13.0 +1100
@@ -73,8 +73,6 @@ extern BOOL tar_reset;
/* clitar bits end */
-static mode_t myumask = 0755;
-
static BOOL prompt = True;
static int printmode = 1;
@@ -2756,8 +2754,6 @@ static void remember_query_host(const ch
pstrcpy(workgroup,lp_workgroup());
load_interfaces();
- myumask = umask(0);
- umask(myumask);
if (getenv(USER)) {
pstrcpy(username,getenv(USER));
This code was apparently copied and pasted into rpctorture.c and is
also redundant there.
--- rpctorture.c.~1.12.~2002-02-04 11:53:12.0 +1100
+++ rpctorture.c2003-02-07 15:35:08.0 +1100
@@ -225,7 +225,6 @@ enum client_action
pstring term_code;
BOOL got_pass = False;
char *cmd_str=;
- mode_t myumask = 0755;
enum client_action cli_action = CLIENT_NONE;
int nprocs = 1;
int numops = 100;
@@ -290,9 +289,6 @@ enum client_action
setup_logging(pname, True);
- myumask = umask(0);
- umask(myumask);
-
if (!get_myname(global_myname))
{
fprintf(stderr, Failed to get my hostname.\n);
--
Martin
Re: [PATCH] umask audit
In addition, wrepld sets its umask to 0. Is that really necessary? -- Martin
Re: Patch for Samba 2.2.6 2.2.7 to work on OS X
On 16 Jan 2003, [EMAIL PROTECTED] wrote: On Thu, Jan 16, 2003 at 01:07:34AM -0500, Michael Bennett wrote: [setregid() fails] So... comments? Does the patch Do The Right Thing? Think this patch or something similar could get merged sometime? :) A patch like that would get accepted immediately. Without it, we have to write the feature test. I think the setuid demystified paper describes how to do setregid() reliably and portably. -- Martin
list filtering
Because of the enormous amount of traffic being generated by Windows viruses[0] I have turned on Mailman attachment filtering on the high-traffic samba.org lists. Lists will now pass only text/plain MIME parts through to the list. multipart/alternative messages with both text and html forms will have the HTML form removed, and messages in only HTML will be squashed to text. Messages which cannot be handled in any of these ways will be rejected. To send patches or log files to the list, you need to either insert them inline into your message, or make sure they're marked as text/plain. On most systems, just making the name be *.txt should be sufficient. I hope everybody's enjoying their SQL Server experience :-) -- Martin samba.org postmaster [0] ... automated notifications about viruses, users complaining about viruses, users complaining about automated notification, users complaining about users complaining, scanners complaining about perfectly ordinary attachments, etc
[patch] winbindd: try to fix 'restrict anonymous=1'
hp CR1501 and friends
This patch tries to make winbindd cope with the security option
'restrict anonymous=1' on NT4 and W2kS. When this option is set, the
DC disallows SAMR calls on unauthenticated connections, but does allow
LSA translations between names and sids.
Obviously winbindd can't be fully functional in this case, but it
ought to be able to still do these operations -- in particular, with
this patch wbinfo -n works, while it does not work without it.
I'm not sure this is right yet but I'd appreciate comments. If this
is correct, I think it ought to be ported to HEAD and 3.0 as well.
It seems to work for me. As Tim suggested I used both built in
(Administrator) and otherwise (jrhacker) SIDs for testing.
This partially reverts the cached failure case, and possibly causes
winbindd to hammer on dcs that just don't want to talk to it. You can
imagine a more detailed fix that specifically detects the ra=1 case
and handles it by using only LSA. From what I know, it doesn't seem
specifically handling that, though perhaps it would be so in HEAD.
Incidentally, gdb remote mode absolutely rocks for debugging appliances.
Thanks to Tim for patient help.
Index: nsswitch/winbindd_cache.c
===
RCS file: /data/cvs/samba/source/nsswitch/winbindd_cache.c,v
retrieving revision 1.5.2.8
diff -u -r1.5.2.8 winbindd_cache.c
--- nsswitch/winbindd_cache.c 31 Oct 2002 23:56:32 - 1.5.2.8
+++ nsswitch/winbindd_cache.c 20 Jan 2003 10:43:58 -
@@ -201,7 +201,8 @@
refresh the domain sequence number. If force is True
then always refresh it, no matter how recently we fetched it
*/
-static void refresh_sequence_number(struct winbindd_domain *domain, BOOL force)
+static NTSTATUS refresh_sequence_number(struct winbindd_domain *domain,
+ BOOL force)
{
NTSTATUS status;
unsigned time_diff;
@@ -210,7 +211,7 @@
/* see if we have to refetch the domain sequence number */
if (!force (time_diff lp_winbind_cache_time())) {
- return;
+ return NT_STATUS_OK;
}
status = wcache-backend-sequence_number(domain, domain-sequence_number);
@@ -238,6 +239,8 @@
DEBUG(10, (refresh_sequence_number: seq number is now %d\n,
domain-sequence_number));
+
+ return status;
}
/*
@@ -276,8 +279,18 @@
TDB_DATA data;
struct cache_entry *centry;
TDB_DATA key;
+ NTSTATUS result;
- refresh_sequence_number(domain, False);
+ result = refresh_sequence_number(domain, False);
+
+ /* Treat an access denied result from refresh_sequence_number as a
+ cache miss. Access denied is returned when the domain
+ controller disallows anonymous access. Perhaps we should treat
+ any error as a miss although that might increase the time it
+ takes winbindd to determine if a domain controller is down. */
+
+ if (NT_STATUS_EQUAL(result, NT_STATUS_ACCESS_DENIED))
+ return NULL;
va_start(ap, format);
smb_xvasprintf(kstr, format, ap);
@@ -738,9 +751,15 @@
do_query:
ZERO_STRUCTP(sid);
- /* Return status value returned by seq number check */
+ /* If the seq number check indicated that there is a problem
+* with this DC, then return that status... except for
+* access_denied. This is special because the dc may be in
+* restrict anonymous = 1 mode, in which case it will deny
+* most unauthenticated operations, but *will* allow the LSA
+* name-to-sid that we try as a fallback. */
- if (!NT_STATUS_IS_OK(domain-last_status))
+ if (!(NT_STATUS_IS_OK(domain-last_status)
+ || NT_STATUS_EQUAL(domain-last_status, NT_STATUS_ACCESS_DENIED)))
return domain-last_status;
status = cache-backend-name_to_sid(domain, name, sid, type);
@@ -784,9 +803,16 @@
do_query:
*name = NULL;
- /* Return status value returned by seq number check */
- if (!NT_STATUS_IS_OK(domain-last_status))
+ /* If the seq number check indicated that there is a problem
+* with this DC, then return that status... except for
+* access_denied. This is special because the dc may be in
+* restrict anonymous = 1 mode, in which case it will deny
+* most unauthenticated operations, but *will* allow the LSA
+* sid-to-name that we try as a fallback. */
+
+ if (!(NT_STATUS_IS_OK(domain-last_status)
+ || NT_STATUS_EQUAL(domain-last_status, NT_STATUS_ACCESS_DENIED)))
return domain-last_status;
status = cache-backend-sid_to_name(domain, mem_ctx, sid, name, type);
--
Martin
Re: smbmount doesn't complain about wrong arguments
On 19 Jan 2003, Christian Jaeger [EMAIL PROTECTED] wrote: Hello I don't have the time to pursue this further at the moment, just wanted to note it: The arguments syntax of smbmount 2.2.3a (smbfs of Debian stable) requires that one gives -o before further options like username=... . Without -o, the username=.. setting is *silently* ignored. I've spent about an hour trying to find out why I couldn't log in now.. I'd say this is a bug. Is there a reason that smbmount doesn't complain? The argument parser is a mess and ought to be fixed. -- Martin
[PATCH] fix pointer error in appl_head loadparm.c
hp CR 1548
This patch fixes a problem I observed where there were error messages
relating to getpwent /usr/bin/passwd, which is obviously a pretty
unlikely username. Investigation showed that the problem comes from
the 1.247.2.52 patch to loadparm.c (appliance_head only), which
replaces some configuration parameters with two-entry arrays
containing both dos and unix values.
The complicated macros for decoding these things has an error which
caused the array indexing to be carried out one level too high in the
tree of pointers. (We got a[1][0] when we wanted a[0][1], basically.)
So the core change is
* (ptr[1])
into
(* ptr)[1]
The attached patch is a minimally-intrusive attempt to fix this. It
also removes some unncessary typecasts which might mask warnings
without apparently doing anything useful. I have to say it still
looks pretty crufty, but I didn't want to risk a larger change, and in
any case this particular problem is only in appliance_head.
It seems that in the current code any caller to the lp_*_dos()
functions will get the wrong value!
Can somebody please review this?
--- loadparm.c.~1.247.2.54.~2003-01-07 16:12:23.0 +1100
+++ loadparm.c 2003-01-17 15:46:36.0 +1100
@@ -1518,28 +1518,38 @@ static char *lp_string(const char *s, BO
/*
- In this section all the functions that are used to access the
- parameters from the rest of the program are defined
+ In this section all the functions that are used to access the
+ parameters from the rest of the program are defined
*/
#define FN_GLOBAL_STRING(fn_name,ptr) \
- char *fn_name(void) {return lp_string(*(char **)(ptr) ? *(char **)(ptr) : ,False);}
+ char *fn_name(void) {return lp_string(*(ptr) ? *(ptr) : ,False);}
+
#define FN_GLOBAL_STRING_UNIX(fn_name,ptr) \
- char *fn_name(void) {return lp_string((*(char **)((ptr))[UNIX_STRING_OFFSET]) ?
*(char **)((ptr)[UNIX_STRING_OFFSET]) : , False);}
+ char *fn_name(void) {return lp_string((*(ptr))[UNIX_STRING_OFFSET] ? (*
+(ptr))[UNIX_STRING_OFFSET] : , False);}
+
+/* ptr is a pointer to an array of 2 pointers to strings */
#define FN_GLOBAL_STRING_DOS(fn_name,ptr) \
- char *fn_name(void) {return lp_string((*(char **)((ptr))[DOS_STRING_OFFSET]) ?
*(char **)((ptr)[DOS_STRING_OFFSET]) : , True);}
+ char *fn_name(void) {return lp_string((*(ptr))[DOS_STRING_OFFSET] ? (*
+(ptr))[DOS_STRING_OFFSET] : , True);}
+
#define FN_GLOBAL_CONST_STRING(fn_name,ptr) \
-const char *fn_name(void) {return(const char *)(*(char **)(ptr) ? *(char **)(ptr) :
);}
+const char *fn_name(void) {return (*(ptr) ? *(ptr) : );}
+
#define FN_GLOBAL_CONST_STRING_UNIX(fn_name,ptr) \
-const char *fn_name(void) {return(const char *)(*(char **)((ptr)[UNIX_STRING_OFFSET])
? *(char **)((ptr)[UNIX_STRING_OFFSET]) : );}
+const char *fn_name(void) {return (*(ptr))[UNIX_STRING_OFFSET] ?
+(*(ptr))[UNIX_STRING_OFFSET] : ;}
+
#define FN_GLOBAL_CONST_STRING_DOS(fn_name,ptr) \
-const char *fn_name(void) {return(const char *)(*(char **)((ptr)[DOS_STRING_OFFSET])
? *(char **)((ptr)[DOS_STRING_OFFSET]) : );}
+const char *fn_name(void) {return (*(ptr))[DOS_STRING_OFFSET] ?
+(*(ptr))[DOS_STRING_OFFSET] : ;}
+
#define FN_GLOBAL_LIST(fn_name,ptr) \
char **fn_name(void) {return(*(char ***)(ptr));}
+
#define FN_GLOBAL_BOOL(fn_name,ptr) \
BOOL fn_name(void) {return(*(BOOL *)(ptr));}
+
#define FN_GLOBAL_CHAR(fn_name,ptr) \
char fn_name(void) {return(*(char *)(ptr));}
+
#define FN_GLOBAL_INTEGER(fn_name,ptr) \
int fn_name(void) {return(*(int *)(ptr));}
--
Martin
msg05409/pgp0.pgp
Description: PGP signature
Re: [PATCH] fix pointer error in appl_head loadparm.c
On 17 Jan 2003, Martin Pool [EMAIL PROTECTED] wrote: It seems that in the current code any caller to the lp_*_dos() functions will get the wrong value! Correction: only the global lp_*dos parameters. Specfically: 1583:FN_GLOBAL_STRING_DOS(lp_serverstring_dos, Globals.szServerString) 1599:FN_GLOBAL_CONST_STRING_DOS(lp_defaultservice_dos, Globals.szDefaultService) 1604:FN_GLOBAL_CONST_STRING_DOS(lp_auto_services_dos, Globals.szAutoServices) 1614:FN_GLOBAL_STRING_DOS(lp_logon_script_dos, Globals.szLogonScript) 1616:FN_GLOBAL_STRING_DOS(lp_logon_path_dos, Globals.szLogonPath) 1619:FN_GLOBAL_STRING_DOS(lp_logon_home_dos, Globals.szLogonHome) So for example, on my PSA, smb.conf wants lp_serverstring_dos() to be set to Hewlett Packard Print Server Appliance, but gdb shows that it's actually . Presumably this is only a cosmetic problem. Similarly for logon_path and logon_home, though I suppose they don't matter for a print-only system. -- Martin
Re: CVS update: samba/source/nsswitch
On 16 Jan 2003, Andrew Bartlett [EMAIL PROTECTED] wrote: Win2k has a bug (feature?) where there is a connection reset if there is a second connection from the SAME IP, before the first session-setup. So an unprivileged process on the client can cause a local denial of service just by repeatedly half-opening connections? Both races need to be protected be separate mutexes. The first should be protected in as generic a manner as possible, due to the fact that it is *any* connection from the IP. Can the connection function be called by a nonprivileged process (say rpcclient or smbclient) on the unix machine? Is so we either need to put the mutex somewhere world-writeable (gross) or just be willing to take our chances without it. -- Martin
Re: recursive mutexes in appl_head winbindd_cm.c?
On 14 Jan 2003, [EMAIL PROTECTED] wrote: get_mutex: ServerReqChallenge ServerAuthenticate2 release_mutex: Yes, that's what we meant. I hypothesized to ab that in NT there is some kind of table indexed by IP (or client name?) holding the challenges. I wonder why? speculation Perhaps there's no per-tcp-connection data area easily accessible from their RPC server, so when it sends out a challenge it can't easily be associated with that TCP socket. I'd guess that if it's a generalized RPC server then the challenge can't be stored on the stack either. The obvious thing, then, is to have a table shared between all threads in the RPC server process, indexed by IP and storing the currently-outstanding challenge. This suggests an alternative implementation for us: rather than using the mutex to make this a critical section, we instead have a machine-wide tdb storing the outstanding challenge from each server. Whoever gets around to authenticating next will remove the challenge from the tdb and use that. Mind you I can't see any way in which this is better than the current approach, and it's probably more fragile. :-) -- Martin
Re: [patch] HEAD winbindd_cm.c mutex bug
On 10 Jan 2003, Andrew Bartlett [EMAIL PROTECTED] wrote:
I think that the connection mutex should be sorted out in
cli_full_connection(), rather than in individual apps. Then we can grab
the mutex for netlogon when operating on that pipe, but I really think
that end should be separate.
Yes, it seems to me that it would be cleaner to do this at a lower
level rather than inside winbindd.
However, for the present do you or jra want to commit this patch? It
does keep it relatively in sync with appliance_head, which I know is
important to jra.
Index: winbindd_cm.c
===
RCS file: /data/cvs/samba/source/nsswitch/winbindd_cm.c,v
retrieving revision 1.59
diff -u -u -p -r1.59 winbindd_cm.c
--- winbindd_cm.c 12 Dec 2002 23:35:55 - 1.59
+++ winbindd_cm.c 15 Jan 2003 05:42:28 -
@@ -369,9 +369,11 @@ static NTSTATUS cm_open_connection(const
new_conn-controller, global_myname(), ipc_domain, ipc_username));
for (i = 0; retry (i 3); i++) {
-
if (!secrets_named_mutex(new_conn-controller, 10)) {
DEBUG(0,(cm_open_connection: mutex grab failed for %s\n,
new_conn-controller));
+ /* try again, but if we never succeed in getting a connection
+then this
+* is the result */
+ result = NT_STATUS_NO_LOGON_SERVERS;
continue;
}
--
Martin
[patch] fix configure bug re yp_get_default_domain
Re-running ./configure (e.g. to change --disable options) in a build directory seems to cause builds to fail with these errors: lib/username.o(.text+0x654): In function `user_in_netgroup_list': : undefined reference to `yp_get_default_domain' lib/access.o(.text+0x163): In function `string_match': : undefined reference to `yp_get_default_domain' smbd/password.o(.text+0x1e9e): In function `check_user_equiv': : undefined reference to `yp_get_default_domain' This happens when I just do make clean in between builds. Tim says the workaround is to remove config.cache, but there are already too many parrots under that carpet. I thought I'd see what was really wrong. The problem seems to be in the yp_get_default_domain detection: the first time configure is run, it detects that it needs -lnsl, but when the cache is present it gets it wrong. Basically the fix is to follow the Autoconf manual and check for the library before checking for the function. This patch is for HEAD, but should apply without too much fuss to APPL_HEAD or 2.2. Index: configure.in === RCS file: /data/cvs/samba/source/configure.in,v retrieving revision 1.384 diff -u -u -p -r1.384 configure.in --- configure.in13 Jan 2003 04:57:21 - 1.384 +++ configure.in13 Jan 2003 05:34:36 - @@ -509,11 +509,7 @@ fi # we need dlopen/dlclose/dlsym/dlerror for PAM, the password database plugins and the plugin loading code -AC_CHECK_FUNCS(dlopen) -if test x$ac_cv_func_dlopen = xno; then -AC_CHECK_LIB(dl, dlopen, [LIBS=$LIBS -ldl; - AC_DEFINE(HAVE_DLOPEN,1,[Whether we have dlopen()])]) -fi +AC_SEARCH_LIBS(dlopen, [dl]) # dlopen/dlclose/dlsym/dlerror will be checked again later and defines will be set then @@ -616,12 +612,8 @@ AC_FUNC_MEMCMP ### # test for where we get crypt() from -AC_CHECK_FUNCS(crypt) -if test x$ac_cv_func_crypt = xno; then -AC_CHECK_LIB(crypt, crypt, [AUTHLIBS=$AUTHLIBS -lcrypt; +AC_SEARCH_LIBS(crypt, [crypt], [AUTHLIBS=$AUTHLIBS -lcrypt; AC_DEFINE(HAVE_CRYPT,1,[Whether the system has the crypt() function])]) -fi - ### # Readline included by default unless explicitly asked not to @@ -730,11 +722,8 @@ fi ### # test for where we get yp_get_default_domain() from +AC_SEARCH_LIBS(yp_get_default_domain, [nsl]) AC_CHECK_FUNCS(yp_get_default_domain) -if test x$ac_cv_func_yp_get_default_domain = xno; then - AC_CHECK_LIB(nsl, yp_get_default_domain, [LIBS=$LIBS -lnsl; - AC_DEFINE(HAVE_YP_GET_DEFAULT_DOMAIN,1,[Whether the system has yp_get_default_domain()])]) -fi # Check if we have execl, if not we need to compile smbrun. AC_CHECK_FUNCS(execl) -- Martin
Re: [patch] fix configure bug re yp_get_default_domain
This is the patch adapted to APPLIANCE_HEAD. Index: configure.in === RCS file: /data/cvs/samba/source/configure.in,v retrieving revision 1.130.2.21 diff -u -u -p -r1.130.2.21 configure.in --- configure.in13 Jan 2003 04:57:01 - 1.130.2.21 +++ configure.in13 Jan 2003 05:49:20 - @@ -588,11 +588,7 @@ fi # we need dlopen/dlclose/dlsym/dlerror for PAM, the password database plugins and the new VFS code -AC_CHECK_FUNCS(dlopen) -if test x$ac_cv_func_dlopen = xno; then - AC_CHECK_LIB(dl, dlopen, [LIBS=$LIBS -ldl; - AC_DEFINE(HAVE_DLOPEN)]) -fi +AC_SEARCH_LIBS(dlopen, [dl]) # dlopen/dlclose/dlsym/dlerror will be checked again later and defines will be set then @@ -790,11 +786,8 @@ fi ### # test for where we get yp_get_default_domain() from +AC_SEARCH_LIBS(yp_get_default_domain, [nsl]) AC_CHECK_FUNCS(yp_get_default_domain) -if test x$ac_cv_func_yp_get_default_domain = xno; then - AC_CHECK_LIB(nsl, yp_get_default_domain, [LIBS=$LIBS -lnsl; - AC_DEFINE(HAVE_YP_GET_DEFAULT_DOMAIN)]) -fi # Check if we have execl, if not we need to compile smbrun. AC_CHECK_FUNCS(execl) @@ -2121,11 +2114,8 @@ AC_ARG_WITH(pam_smbpass, # test for where we get crypt() from, but only # if not using PAM if test $with_pam_for_crypt = no; then +AC_SEARCH_LIBS(crypt, [crypt]) AC_CHECK_FUNCS(crypt) -if test x$ac_cv_func_crypt = xno; then -AC_CHECK_LIB(crypt, crypt, [LIBS=$LIBS -lcrypt; - AC_DEFINE(HAVE_CRYPT)]) -fi fi ## -- Martin
Re: [patch] fix configure bug re yp_get_default_domain
By the way, these both work with autoconf2.13 and 2.57. -- Martin
Re: [PATCH] audit handling of waitpid() status codes
On 9 Jan 2003, [EMAIL PROTECTED] wrote:
Thanks for checking it.
Part of this (the smbd/chgpasswd.c patch) is incorrect I think.
You have changed the line :
if (WIFEXITED(wstat) == 0) {
...
return False;
}
to
if (WIFEXITED(wstat)) {
...
return False;
}
The man page states :
WIFEXITED(status)
is non-zero if the child exited normally.
exited normally in this context means called _exit(), rather than
being terminated by a signal. It doesn't necessarily mean exited 0.
To determine that you need to evaluate WIFEXITED(s) (WEXITSTATUS(s)
== 0).
This particular clause is meant to catch an error condition
(not a normal exit). I agree it's not good code and could be cleaned
up but this change reverses the return code on password change
success.
You're right, I misunderstood what it was trying to do, because the
process exited while we were waiting message is printed only when
it's not true. Here's an updated patch which corrects the messages
and returns the same values.
Index: client/smbmount.c
===
RCS file: /data/cvs/samba/source/client/smbmount.c,v
retrieving revision 1.57
diff -u -u -r1.57 smbmount.c
--- client/smbmount.c 13 Nov 2002 02:21:55 - 1.57
+++ client/smbmount.c 9 Jan 2003 23:11:13 -
@@ -79,7 +79,11 @@
break;
}
/* If we get here - the child exited with some error status */
- exit(status);
+ if (WIFSIGNALLED(status)) {
+ exit(128 + WTERMSIG(status));
+ } else {
+ exit(WEXITSTATUS(status));
+ }
}
signal( SIGTERM, SIG_DFL );
@@ -499,6 +503,9 @@
if (WIFEXITED(status) WEXITSTATUS(status) != 0) {
fprintf(stderr,smbmnt failed: %d\n, WEXITSTATUS(status));
/* FIXME: do some proper error handling */
+ exit(1);
+ } else if (WIFSIGNALLED(status)) {
+ fprintf(stderr, smbmnt killed by signal %d\n, WTERMSIG(status));
exit(1);
}
Index: lib/smbrun.c
===
RCS file: /data/cvs/samba/source/lib/smbrun.c,v
retrieving revision 1.20
diff -u -u -r1.20 smbrun.c
--- lib/smbrun.c28 Jul 2002 02:20:15 - 1.20
+++ lib/smbrun.c9 Jan 2003 23:11:13 -
@@ -130,6 +130,11 @@
return WEXITSTATUS(status);
}
#endif
+#if defined(WIFSIGNALLED) defined(WTERMSIG)
+ if (WIFSIGNALLED(status)) {
+ return 128 + WTERMSIG(status);
+ }
+#endif
return status;
}
Index: lib/util_file.c
===
RCS file: /data/cvs/samba/source/lib/util_file.c,v
retrieving revision 1.36
diff -u -u -r1.36 util_file.c
--- lib/util_file.c 28 Jun 2002 03:19:20 - 1.36
+++ lib/util_file.c 9 Jan 2003 23:11:13 -
@@ -362,7 +362,7 @@
while ((n = read(fd, buf, sizeof(buf))) 0) {
tp = Realloc(p, total + n + 1);
if (!tp) {
- DEBUG(0,(file_pload: failed to exand buffer!\n));
+ DEBUG(0,(file_pload: failed to expand buffer!\n));
close(fd);
SAFE_FREE(p);
return NULL;
@@ -372,6 +372,8 @@
}
if (p) p[total] = 0;
+ /* FIXME: Perhaps ought to check that the command completed
+* successfully; if not the data may be truncated. */
sys_pclose(fd);
if (size) *size = total;
Index: smbd/chgpasswd.c
===
RCS file: /data/cvs/samba/source/smbd/chgpasswd.c,v
retrieving revision 1.98
diff -u -u -r1.98 chgpasswd.c
--- smbd/chgpasswd.c9 Jan 2003 06:58:07 - 1.98
+++ smbd/chgpasswd.c9 Jan 2003 23:11:14 -
@@ -408,20 +408,22 @@
(We were waiting for the wrong process ID\n));
return (False);
}
- if (WIFEXITED(wstat) == 0)
+
+ if (WIFEXITED(wstat) WEXITSTATUS(wstat) != 0)
{
DEBUG(3,
- (The process exited while we were waiting\n));
+ (The process exited with code %d while we were
+waiting\n,
+ WEXITSTATUS(wstat)));
return (False);
}
- if (WEXITSTATUS(wstat) != 0)
+ else if (WIFSIGNALED(wstat))
{
DEBUG(3,
- (The status of the process exiting was %d\n,
- wstat));
+ (The process was killed by
recursive mutexes in appl_head winbindd_cm.c?
I'm looking at jra's 1.33.2.16 change to winbindd_cmd.c in relation to
hp CR1501.
I think there are some problems with the way the mutex reference count
is handled. I'm not sure what is the cleanest way to fix it.
The mutexes are implemented on top of fcntl locks, which cannot be
nested. Therefore winbindd holds an in-memory reference count for
each lock. When this increments from zero, the OS lock is taken; when
it decreases to zero the OS lock is released. So far so good.
jra, can you explain what the recursion thing in this patch is for?
Tim says the point of the mutex is to protect against an NT bug that
causes failures if more than one connection tries to authenticate at
the same time.
In cm_open_connection:
for (i = 0; retry (i NUM_CLI_AUTH_CONNECT_RETRIES); i++) {
if (!secrets_named_mutex(new_conn-controller,
WINBIND_SERVER_MUTEX_WAIT_TIME, new_conn-mutex_ref_count)) {
DEBUG(0,(cm_open_connection: mutex grab failed for %s\n,
new_conn-controller));
continue;
}
result = cli_full_connection(new_conn-cli, global_myname_unix(),
new_conn-controller,
dc_ip, 0, CLI_AUTH_TIMEOUT, IPC$,
IPC, ipc_username, ipc_domain,
ipc_password, strlen(ipc_password), retry);
if (NT_STATUS_IS_OK(result))
break;
}
If we fail to acquire the mutex, then we continue trying a few times,
which is probably OK. However, if we never get the mutex after three
times, then the loop terminates and we proceed on through the function
with 'result' uninitialized, which would cause trouble.
In another case, suppose that our first attempt to call
cli_full_connection() fails. (I think this is the case I'm seeing --
because of something to do with restrict anonymous, we can't get in
to the PDC.) We therefore end up with 3 acquisitions of the mutex,
and one of them is released when we exit the function, so the fctnl
lock is never freed, which presumably causes trouble with other things
later -- we have leaked two mutex reservations.
One way to cope better would be for the function to fail if it doesn't
get the mutex after the timeout. However, since the mutex is only a
safeguard against an NT bug, we might be better off taking our chances
and proceeding anyhow -- this is what the code does at the moment.
However, it still tries to release the mutex even though it was not
actually acquired. This causes panic()s in secrets.c.
cm_open_connection and get_connection_from_cache in appliance_head
both have a keep_mutex flag that is used by cm_get_netlogon_cli to
hold onto the mutex for a longer period so that it can also guard the
NetLogon phase. There seem to be two problems with this.
If the connection is returned from cache, then the mutex count is in
fact not acquired, and it is incorrect for cm_get_netlogon_cli() to
release it:
if (conn-mutex_ref_count)
secrets_named_mutex_release(conn-controller, conn-mutex_ref_count);
Examining the refcount seems to me not to be a safe protection against
this: it might already be 1 because some other caller has acquired
it, but that doesn't mean we have the right to release it. We're
giving up somebody else's lock. This happens in a couple of places.
Also, as noted above, sometimes cm_open_connection() completes without
acquiring the mutex, but cm_get_netlogon_cli() assumes that it's
always taken.
--
Martin
[patch] HEAD winbindd_cm.c mutex bug
This patch is meant to fix the case where we repeatedly fail to
acquire the mutex for opening the connection. At the moment the code
proceeds with neither the new_conn- or result variables initialized,
which I'm pretty sure is a bug.
I don't know if this is the most appropriate status code but it should
be set to something.
--- winbindd_cm.c.~1.59.~ 2003-01-09 12:11:32.0 +1100
+++ winbindd_cm.c 2003-01-10 15:55:53.0 +1100
@@ -369,9 +369,11 @@ static NTSTATUS cm_open_connection(const
new_conn-controller, global_myname(), ipc_domain, ipc_username));
for (i = 0; retry (i 3); i++) {
-
if (!secrets_named_mutex(new_conn-controller, 10)) {
DEBUG(0,(cm_open_connection: mutex grab failed for %s\n,
new_conn-controller));
+ /* try again, but if we never succeed in getting a connection
+then this
+* is the result */
+ result = NT_STATUS_POSSIBLE_DEADLOCK;
continue;
}
--
Martin
Re: recursive mutexes in appl_head winbindd_cm.c?
Here's my idea for fixing this in appliance-head, without reworking
the mutex reference count.
Basically it tries to
- avoid undefined behaviour in the case where we fail to acquire the
mutex
- avoid leaking locks in the case where we fail to connect to the
server
- avoid releasing the mutex more times than it has been acquired,
because this causes a panic
I haven't tested this in place yet, but I thought I'd send it in the
hope that jra could tell me if I'm on the right track.
Index: winbindd_cm.c
===
RCS file: /data/cvs/samba/source/nsswitch/winbindd_cm.c,v
retrieving revision 1.33.2.19
diff -u -u -p -r1.33.2.19 winbindd_cm.c
--- winbindd_cm.c 10 Dec 2002 00:50:28 - 1.33.2.19
+++ winbindd_cm.c 10 Jan 2003 06:27:09 -
@@ -45,6 +45,22 @@
*/
/*
+ The per-server mutex on opening server connections is required to
+ work around a suspected bug in NT, which causes failures if the same
+ client host tries to authenticate on two connections at the same
+ time.
+
+ In addition, the mutex is still held after opening the connection
+ when trying to do a NetLogon.
+
+ If we fail to acquire the mutex because somebody else is hogging it,
+ then we can still proceed to open the connection and we take our
+ chances with NT. However we must then be careful not to release it.
+
+ This whole mechanism is quite different in HEAD.
+*/
+
+/*
TODO:
- I'm pretty annoyed by all the make_nmb_name() stuff. It should be
@@ -68,7 +84,12 @@ struct winbindd_cm_conn {
fstring domain;
fstring controller;
fstring pipe_name;
+
+ /** Tells how many callers inside this process are using the
+* lock on connections to this server. When 0, the
+* system-wide mutex in the tdb is released. **/
size_t mutex_ref_count;
+
struct cli_state *cli;
POLICY_HND pol;
};
@@ -163,10 +184,16 @@ static void add_failed_connection_entry(
-/* Open a connction to the remote server, cache failures for 30 seconds */
-
+/**
+ * Open a connection to the remote server, cache failures for 30 seconds
+ *
+ * @param keep_mutex If true, a reservation on the server mutex is
+ * still held on successful return, so that the caller can use it and
+ * release it later.
+ **/
static NTSTATUS cm_open_connection(const char *domain, const int pipe_index,
- struct winbindd_cm_conn *new_conn, BOOL keep_mutex)
+ struct winbindd_cm_conn *new_conn,
+ BOOL keep_mutex)
{
struct failed_connection_cache *fcc;
NTSTATUS result;
@@ -228,13 +255,15 @@ static NTSTATUS cm_open_connection(const
DEBUG(5, (connecting to %s from %s with username [%s]\\[%s]\n,
new_conn-controller, global_myname_unix(), ipc_domain, ipc_username));
+ if (!secrets_named_mutex(new_conn-controller, WINBIND_SERVER_MUTEX_WAIT_TIME,
+new_conn-mutex_ref_count)) {
+ DEBUG(0,(cm_open_connection: mutex grab failed for %s\n,
+new_conn-controller));
+ /* continue anyway; note that the mutex may not actually be
+* held during the rest of this function. */
+ }
+
for (i = 0; retry (i NUM_CLI_AUTH_CONNECT_RETRIES); i++) {
-
- if (!secrets_named_mutex(new_conn-controller,
WINBIND_SERVER_MUTEX_WAIT_TIME, new_conn-mutex_ref_count)) {
- DEBUG(0,(cm_open_connection: mutex grab failed for %s\n,
new_conn-controller));
- continue;
- }
-
result = cli_full_connection(new_conn-cli, global_myname_unix(),
new_conn-controller,
dc_ip, 0, CLI_AUTH_TIMEOUT, IPC$,
IPC, ipc_username, ipc_domain,
@@ -249,7 +278,8 @@ static NTSTATUS cm_open_connection(const
SAFE_FREE(ipc_password);
if (!NT_STATUS_IS_OK(result)) {
- secrets_named_mutex_release(new_conn-controller,
new_conn-mutex_ref_count);
+ if (new_conn-mutex_ref_count 0)
+ secrets_named_mutex_release(new_conn-controller,
+new_conn-mutex_ref_count);
add_failed_connection_entry(new_conn, result);
return result;
}
@@ -264,15 +294,19 @@ static NTSTATUS cm_open_connection(const
* if the PDC is an NT4 box. but since there is only one 2k
* specific UUID right now, i'm not going to bother. --jerry
*/
- secrets_named_mutex_release(new_conn-controller,
new_conn-mutex_ref_count);
+ if (new_conn-mutex_ref_count 0)
+ secrets_named_mutex_release(new_conn-controller,
+new_conn-mutex_ref_count);
if ( !is_win2k_pipe(pipe_index) )
[PATCH] audit handling of waitpid() status codes
I found a data-corruption bug in ccache a few weeks ago relating to
incorrect handling of wait() status codes, so I thought I would do a
quick check for similar things in Samba.
A patch is included:
- several cases where child process failure is not detected
- one inverted boolean
- better messages when child processes crash
- one fixme I easily see how to handle
- one typo
I think the external behaviour is otherwise the same.
Could somebody please review this?
--
Martin
cvs server: Diffing .
cvs server: Diffing aparser
cvs server: Diffing aparser/templates
cvs server: Diffing auth
cvs server: Diffing bin
cvs server: Diffing client
Index: client/smbmount.c
===
RCS file: /data/cvs/samba/source/client/smbmount.c,v
retrieving revision 1.57
diff -u -r1.57 smbmount.c
--- client/smbmount.c 13 Nov 2002 02:21:55 - 1.57
+++ client/smbmount.c 9 Jan 2003 06:09:08 -
@@ -79,7 +79,11 @@
break;
}
/* If we get here - the child exited with some error status */
- exit(status);
+ if (WIFSIGNALLED(status)) {
+ exit(128 + WTERMSIG(status));
+ } else {
+ exit(WEXITSTATUS(status));
+ }
}
signal( SIGTERM, SIG_DFL );
@@ -499,6 +503,9 @@
if (WIFEXITED(status) WEXITSTATUS(status) != 0) {
fprintf(stderr,smbmnt failed: %d\n, WEXITSTATUS(status));
/* FIXME: do some proper error handling */
+ exit(1);
+ } else if (WIFSIGNALLED(status)) {
+ fprintf(stderr, smbmnt killed by signal %d\n, WTERMSIG(status));
exit(1);
}
cvs server: Diffing codepages
cvs server: Diffing groupdb
cvs server: Diffing include
cvs server: Diffing intl
cvs server: Diffing lib
Index: lib/smbrun.c
===
RCS file: /data/cvs/samba/source/lib/smbrun.c,v
retrieving revision 1.20
diff -u -r1.20 smbrun.c
--- lib/smbrun.c28 Jul 2002 02:20:15 - 1.20
+++ lib/smbrun.c9 Jan 2003 06:09:08 -
@@ -130,6 +130,11 @@
return WEXITSTATUS(status);
}
#endif
+#if defined(WIFSIGNALLED) defined(WTERMSIG)
+ if (WIFSIGNALLED(status)) {
+ return 128 + WTERMSIG(status);
+ }
+#endif
return status;
}
Index: lib/util_file.c
===
RCS file: /data/cvs/samba/source/lib/util_file.c,v
retrieving revision 1.36
diff -u -r1.36 util_file.c
--- lib/util_file.c 28 Jun 2002 03:19:20 - 1.36
+++ lib/util_file.c 9 Jan 2003 06:09:08 -
@@ -362,7 +362,7 @@
while ((n = read(fd, buf, sizeof(buf))) 0) {
tp = Realloc(p, total + n + 1);
if (!tp) {
- DEBUG(0,(file_pload: failed to exand buffer!\n));
+ DEBUG(0,(file_pload: failed to expand buffer!\n));
close(fd);
SAFE_FREE(p);
return NULL;
@@ -372,6 +372,8 @@
}
if (p) p[total] = 0;
+ /* FIXME: Perhaps ought to check that the command completed
+* successfully; if not the data may be truncated. */
sys_pclose(fd);
if (size) *size = total;
cvs server: Diffing libads
cvs server: Diffing libsmb
cvs server: Diffing locking
cvs server: Diffing msdfs
cvs server: Diffing nmbd
cvs server: Diffing nsswitch
cvs server: Diffing pam_smbpass
cvs server: Diffing pam_smbpass/samples
cvs server: Diffing param
cvs server: Diffing passdb
cvs server: Diffing po
cvs server: Diffing popt
cvs server: Diffing printing
cvs server: Diffing profile
cvs server: Diffing python
cvs server: Diffing python/examples
cvs server: Diffing python/examples/spoolss
cvs server: Diffing python/examples/tdbpack
cvs server: Diffing python/samba
cvs server: Diffing registry
cvs server: Diffing rpc_client
cvs server: Diffing rpc_parse
cvs server: Diffing rpc_server
cvs server: Diffing rpcclient
cvs server: Diffing sam
cvs server: Diffing script
cvs server: Diffing smbd
Index: smbd/chgpasswd.c
===
RCS file: /data/cvs/samba/source/smbd/chgpasswd.c,v
retrieving revision 1.97
diff -u -r1.97 chgpasswd.c
--- smbd/chgpasswd.c8 Jan 2003 07:02:18 - 1.97
+++ smbd/chgpasswd.c9 Jan 2003 06:09:09 -
@@ -408,20 +408,20 @@
(We were waiting for the wrong process ID\n));
return (False);
}
- if (WIFEXITED(wstat) == 0)
+ if (WIFEXITED(wstat))
{
DEBUG(3,
- (The process exited while we were waiting\n));
+ (The
Urgent Unix Support Requirement for Frankfurt (fwd from j.schroeder@rockwelldatacorp.com)
From: J Schroeder [EMAIL PROTECTED] Subject: Urgent Unix Support Requirement for Frankfurt Date: Wed, 27 Nov 2002 13:21:37 +0100 Hi. If any of you guys are looking (or know of anyone looking) for a new position in Frankfurt, I have a colleague looking for several Unix Support people there. Please drop me a mail if interested and I will forward details The rquirement involves: Knowledge of UNIX, SQL or programming languages, Standard Microsoft software, Native German speaker (also good knowledge of English) Best regards, J. Schroeder
(fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
I'll write up a short page describing how to use them, unless Jerry particularly wants to do it. - Forwarded message from [EMAIL PROTECTED] - From: [EMAIL PROTECTED] Subject: Suggestion: describe (or link to) how to verify your distributions Date: Fri, 22 Nov 2002 20:21:38 GMT To: [EMAIL PROTECTED] Hi folks, Thanks for all your work. Thanks for taking the time to secure it and to distribute it in a secure fashion. Today as I downloaded your new version, aware of the openssh trojan and aware that MD5 signatures hosted on the same server doesn't verify anything, I was pleased to find a digital signature for samba. A suggestion though. In addition to providing the digital signature it would be great if you could include a few links or a page or two describing how to use it. I ask this, because I can't figure out how to get PGP to use your signature. And having visited CERT, PGP, GPG, and using google, I am still stumped as to what to do with this detached digital signature. You folks are one of the most important projects around. It's terrific that you are distributing digital signatures, you could improve on that a bit by distributing information on how to use that digital signature. Thank you, Jerry Asher - End forwarded message - -- Martin msg04549/pgp0.pgp Description: PGP signature
Re: (fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
On 22 Nov 2002, Steve Langasek [EMAIL PROTECTED] wrote: On Fri, Nov 22, 2002 at 12:56:39PM -0800, Martin Pool wrote: I'll write up a short page describing how to use them, unless Jerry particularly wants to do it. In five words or less, from the gpg manpage: $ gpg --verify samba-2.2.7.tar.gz.asc samba-2.2.7.tar.gz Yeah, sure, but: What does this all mean? Why should I care? Where do I get GPG? Where do I get the samba codesigning key? How do I import it? How do I know I got the right one? What do I do if it doesn't verify? etc... -- Martin
(fwd from jerry@theashergroup.com) security suggestion continued...
- Forwarded message from [EMAIL PROTECTED] - From: [EMAIL PROTECTED] Subject: security suggestion continued... Date: Fri, 22 Nov 2002 21:01:35 GMT To: [EMAIL PROTECTED] Cc: [EMAIL PROTECTED] Following up my prior message, I actually found a nice reference on how to verify samba distributions. That reference was written by David Lechnyr and can be found http://hr.uoregon.edu/davidrl/samba.html I've appended David's content below. Anyway my point being that somewhere on your download pages, it would be very helpful to describe how to use your signatures. And it turns out, I don't believe one can use the gui version of free PGP to do this. Maybe with the command line version of pgp... Thanks, Jerry From http://hr.uoregon.edu/davidrl/samba.html Installing Samba It's important to run the latest version of Samba. For example, a security hole has been discovered in versions 2.2.2 through 2.2.6 of Samba that could potentially allow an attacker to gain root access on the target machine. It pays to stay up to date ;-) Download the files: $ wget http://us1.samba.org/samba/ftp/samba-2.2.7.tar.gz $ wget http://us1.samba.org/samba/ftp/samba-2.2.7.tar.gz.asc These days, it's strongly recommended that you verify the PGP signature for any source file before installing it. Download the Samba PGP Public Key file from http://us1.samba.org/samba/ftp/samba-pubkey.asc and run: $ gpg --import samba-pubkey.asc $ gpg --verify samba-2.2.7.tar.gz.asc If you receive a message like, Good signature from Samba Distribution Verification Key... then all is well. The warnings about trust relationships can be ignored. An example of what you would not want to see would be: gpg: Signature made Mon Aug 26 19:06:30 2002 PDT using RSA key ID 628E0A02 gpg: BAD signature from Samba Distribution Verification Key - End forwarded message - -- Martin
Re: (fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
On 22 Nov 2002, Martin Pool [EMAIL PROTECTED] wrote: On 22 Nov 2002, Steve Langasek [EMAIL PROTECTED] wrote: On Fri, Nov 22, 2002 at 12:56:39PM -0800, Martin Pool wrote: I'll write up a short page describing how to use them, unless Jerry particularly wants to do it. In five words or less, from the gpg manpage: $ gpg --verify samba-2.2.7.tar.gz.asc samba-2.2.7.tar.gz Yeah, sure, but: What does this all mean? Why should I care? Where do I get GPG? Where do I get the samba codesigning key? How do I import it? How do I know I got the right one? What do I do if it doesn't verify? etc... Before you reply: I know the answers to these, but probably many people don't. Merely saying how to run the command is not a complete solution -- using GPG without understanding at least the basics is worse than not using it at all. -- Martin
Re: (fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
On 22 Nov 2002, Steve Langasek [EMAIL PROTECTED] wrote: Hmm. I see nine signatures already, and I have a full trust relationship to the key which traverses multiple paths through the keyring, the shortest of which is only three hops long, despite never having met a member of the Samba Team. All in all, a well-connected key, and I think if there are people who get this error and actually care about it :), the problem is more likely to lie on their end of the web of trust. According to samba.html, the distribution key is http://us1.samba.org/samba/ftp/samba-pubkey.asc gpg: key 2F87AF6F: public key Samba Distribution Verification Key [EMAIL PROTECTED] This has only a single signature, from Jerry. mbp@toey ~% gpg --list-sig 2F87AF6F pub 1024D/2F87AF6F 2002-10-15 Samba Distribution Verification Key [EMAIL PROTECTED] sig 3 2F87AF6F 2002-10-15 Samba Distribution Verification Key [EMAIL PROTECTED] sig D83511F6 2002-10-15 Gerald W. Carter [EMAIL PROTECTED] sub 1024g/4A271F85 2002-10-15 [expires: 2004-10-14] sig 2F87AF6F 2002-10-15 Samba Distribution Verification Key [EMAIL PROTECTED] Jerry's key is pretty well signed, but perhaps not strongly connected to the world at large. I don't know of any way to get GPG to automatically download signatures for the web of trust, so unless people happen to have Jerry's key and those of the people who certify him it is likely to be untrusted. I think it would be good to get other developers to sign the distribution key. Perhaps we might also get organizations like CERT or AusCERT to sign the key (if they will), because administrators are likely to already have their pubkeys. Jerry, if you can call Sundeep's desk then I will listen to your voice and sign your key. -- Martin msg04562/pgp0.pgp Description: PGP signature
Re: (fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
Incidentally, this form is pretty useful when trying to establish the
validity of a key. It would be nice if it were available from a GUI.
gpg --list-sig A0B3E88B|awk '/id not found/ { print $2 }' |sort -u |xargs gpg
--recv-key
--
Martin
msg04563/pgp0.pgp
Description: PGP signature
Re: (fwd from jerry@theashergroup.com) Suggestion: describe (or link to) how to verify your distributions
On 22 Nov 2002, Steve Langasek [EMAIL PROTECTED] wrote: On Fri, Nov 22, 2002 at 03:16:09PM -0600, David W. Chapman Jr. wrote: On Fri, Nov 22, 2002 at 01:08:39PM -0800, Martin Pool wrote: Yeah, sure, but: What does this all mean? Why should I care? Where do I get GPG? Where do I get the samba codesigning key? How do I import it? How do I know I got the right one? What do I do if it doesn't verify? I always wondered if someone uploaded a tarball with a trojan, what's preventing them from updating the .asc file as well? The signature file can only be produced by somebody who has the private key, which (I hope) only resides on well-secured machines separate from the distribution machine. For example it might be on a PC at Jerry's house. It's a cryptographic signature that can only be produced using a specific key. Assuming that the key belongs to the party whose name is on it, and assuming that the key is well-protected from theft, and assuming that the algorithms used by PGP haven't been broken, you can be assured that the signature was made by the person it claims to have come from. So the failure modes are: 1 - Somebody breaks into Jerry or some other signer's PC, and from there to samba.org. Equivalently, Jerry's laptop is stolen by somebody smart enough to understand what they found. (Don't take keys to DEFCON!) 2 - Somebody uploads an invalid .asc file, but nobody actually checks it, or at least nobody raises the alarm for some time. 3 - Somebody changed the .tgz, .asc, and also the key stored on the same keyserver. The key is signed with what look like plausible signatures. Again, this will eventually be detected, but perhaps not until some trouble is caused. 4 - GPG is broken. (By far the least likely.) -- Martin
HP/UX portability guidelines
I found these while investigating a distcc portability bug. Anyone interested in getting stuff running on HP/UX might find it interesting. http://devrsrc1.external.hp.com/STKL/inhibitors.html -- Martin You see, in the unix world, system means a bunch of unrelated programs. -- Steve Strassman
[PATCH] fix bug in lpq parsing
This patch merges some missing code from 1.2.4.4 on APPLIANCE_HEAD
into head, fixing a bug in the parser for lpq output.
Basically we're trying to concatenate several fields into a single
string, but the calculation of the amount of space remaining is wrong.
This causes a crash when there are a lot of fields in the output,
because a negative value can be passed as the length parameter to
safe_strcat.
This was originally HP Nautilus CR #430.
Please let me know if it's OK.
Index: lpq_parse.c
===
RCS file: /data/cvs/samba/source/printing/lpq_parse.c,v
retrieving revision 1.11
diff -u -u -r1.11 lpq_parse.c
--- lpq_parse.c 2002/03/15 08:14:06 1.11
+++ lpq_parse.c 2002/04/19 07:28:02
-149,21 +149,17
StrnCpy(buf-fs_file,tok[FILETOK],sizeof(buf-fs_file)-1);
if ((FILETOK + 1) != TOTALTOK) {
-int bufsize;
int i;
-bufsize = sizeof(buf-fs_file) - strlen(buf-fs_file) - 1;
-
for (i = (FILETOK + 1); i TOTALTOK; i++) {
- safe_strcat(buf-fs_file, ,bufsize);
- safe_strcat(buf-fs_file,tok[i],bufsize - 1);
- bufsize = sizeof(buf-fs_file) - strlen(buf-fs_file) - 1;
- if (bufsize = 0) {
-break;
- }
+ /* FIXME: Using fstrcat rather than other means is a bit
+ * inefficient; this might be a problem for enormous queues with
+ * many fields. */
+ fstrcat(buf-fs_file, );
+ fstrcat(buf-fs_file, tok[i]);
}
/* Ensure null termination. */
-buf-fs_file[sizeof(buf-fs_file)-1] = '\0';
+fstrterminate(buf-fs_file);
}
#ifdef PRIOTOK
-282,21 +278,17
StrnCpy(buf-fs_file,tokarr[LPRNG_FILETOK],sizeof(buf-fs_file)-1);
if ((LPRNG_FILETOK + 1) != LPRNG_TOTALTOK) {
-int bufsize;
int i;
-bufsize = sizeof(buf-fs_file) - strlen(buf-fs_file) - 1;
-
for (i = (LPRNG_FILETOK + 1); i LPRNG_TOTALTOK; i++) {
- safe_strcat(buf-fs_file, ,bufsize);
- safe_strcat(buf-fs_file,tokarr[i],bufsize - 1);
- bufsize = sizeof(buf-fs_file) - strlen(buf-fs_file) - 1;
- if (bufsize = 0) {
-break;
- }
+ /* FIXME: Using fstrcat rather than other means is a bit
+ * inefficient; this might be a problem for enormous queues with
+ * many fields. */
+ fstrcat(buf-fs_file, );
+ fstrcat(buf-fs_file, tokarr[i]);
}
/* Ensure null termination. */
-buf-fs_file[sizeof(buf-fs_file)-1] = '\0';
+fstrterminate(buf-fs_file);
}
return(True);
--
Martin
[PATCH] backport lprng_time parsing to appl-head
This patch backports LPRng_time() from HEAD to APPLIANCE_HEAD without
modification. As the comment says, it allows Samba to parse LPRng
output that supplies either just a time, or a date and time.
(I believe this fixes HP CR 594, because the lpr on that box supplies
a date.)
I'm not committing this directly because I want to be really careful
about that branch. Could Tim, Jerry or jra let me know if you think
it's OK?
Thanks,
--
Martin
--- lpq_parse.c.~1.2.4.6.~ Fri Apr 19 17:13:30 2002
+++ lpq_parse.c Fri Apr 19 17:34:24 2002
@@ -180,20 +180,39 @@ the lpq output. The lpq time looks like
[EMAIL PROTECTED] June 30, 1998.
Modified to work with the re-written parse_lpq_lprng routine.
+
+[EMAIL PROTECTED] Dec 17,1999
+Modified to work with lprng 3.16
+With lprng 3.16 The lpq time looks like
+ 23:15:07
+ 23:15:07.100
+ 1999-12-16-23:15:07
+ 1999-12-16-23:15:07.100
+
*/
static time_t LPRng_time(char *time_string)
{
- time_t jobtime;
- struct tm *t;
+ time_t jobtime;
+ struct tm t;
+
+ jobtime = time(NULL); /* default case: take current time */
+ t = *localtime(jobtime);
- jobtime = time(NULL); /* default case: take current time */
- t = localtime(jobtime);
- t-tm_hour = atoi(time_string);
- t-tm_min = atoi(time_string+3);
- t-tm_sec = atoi(time_string+6);
- jobtime = mktime(t);
+ if ( atoi(time_string) 24 ){
+ t.tm_hour = atoi(time_string);
+ t.tm_min = atoi(time_string+3);
+ t.tm_sec = atoi(time_string+6);
+ } else {
+ t.tm_year = atoi(time_string)-1900;
+ t.tm_mon = atoi(time_string+5)-1;
+ t.tm_mday = atoi(time_string+8);
+ t.tm_hour = atoi(time_string+11);
+ t.tm_min = atoi(time_string+14);
+ t.tm_sec = atoi(time_string+17);
+ }
+ jobtime = mktime(t);
- return jobtime;
+ return jobtime;
}
