[Patches] [ python-Patches-633013 ] Fix NIS causing interpreter core dump
noreply@sourceforge.net
noreply@sourceforge.net
Mon, 04 Nov 2002 15:06:55 -0800
Patches item #633013, was opened at 2002-11-03 22:44
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=633013&group_id=5470
Category: Core (C code)
Group: Python 2.2.x
Status: Open
>Resolution: Accepted
Priority: 5
Submitted By: Neal Norwitz (nnorwitz)
Assigned to: Nobody/Anonymous (nobody)
Summary: Fix NIS causing interpreter core dump
Initial Comment:
When running on the Compaq test drive machines,
test_nis will cause the interpreter to core dump. The
attached patch prevents the core dump which is caused
by passing a negative value to
PyString_FromStringAndSize(). I'm not sure if it's
100% correct, but the test passes and the interpreter
doesn't core dump.
Any one else know if this is correct? I'll apply to
prevent the core dump, unless someone complains.
----------------------------------------------------------------------
>Comment By: Martin v. Löwis (loewis)
Date: 2002-11-05 00:06
Message:
Logged In: YES
user_id=21627
I think this patch is fine. My understanding is that key and
value have either both or neither a terminating null, but
treating them separately should work just as well.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-11-05 00:02
Message:
Logged In: YES
user_id=33168
Are key & value independant? This code works for me:
if (indata->fix) {
if (inkeylen > 0 && inkey[inkeylen-1] == '\0')
inkeylen--;
if (invallen > 0 && inval[invallen-1] == '\0))
invallen--;
}
Does this work for you or would you rather see the checks
together? The reason why I did it this way was in case key
or value was a problem, but not both. I don't know if this
is a valid concern or not. Let me know how you want me to
proceed. If you want to take this over, that's fine too,
since the only NIS machine I can test on is the test drive
machines.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-11-04 14:41
Message:
Logged In: YES
user_id=21627
I was thinking of
if (indata->fix && inkeylen >0 && invaluelen>0
&& data[inkeylen-1] == '\0' && value[invaluelen-1] == '\0){
inkeylen--; invaluelen--;
}
That there is a '':'' entry in mail.aliases might be a bug
in the NIS configuration of testdrive. However, the problem
with "sometimes it the length includes the null, sometimes
not" is probably independent from this specific installation.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-11-04 14:10
Message:
Logged In: YES
user_id=33168
One thing to note: I believe I originally found this
problem on the Alphas (Tru64) even before the merger with
HP. So this problem could deal more with the NIS configuration.
I'm not sure I understand, do you want something like this:
if (indata->fix) {
if (data[datalen] != '\0')
datalen--;
}
Where data/datalen would be done for both inkey and inval?
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-11-04 08:15
Message:
Logged In: YES
user_id=21627
I now have an account on the testdrive machine (as you just
referred to them). The -1 is not coming from yp_all; foreach
is creating it itself, since fix is true.
There appears to be something strange with mail.aliases: On
some systems, keys and values have a null byte, on others,
they don't. In JNDI, Sun could not solve this in any other
way but defining a property com.sun.jndi.nis.mailaliases
which indicates whether a null byte should be assumed to be
there, see
http://www-iiuf.unifr.ch/iiufdev/doc/public/jndi/providers/jndi-nis.html
I would then suggest a different strategy: If fix is set,
and both the key and the value have a terminating null
included in their length, ignore that. Otherwise, copy all
bytes into key and value.
I'm still uncertain what the '':'' pair is supposed to
indicate, but it appears that mail.aliases deliberately
includes "invalid" entries. For example, sendmail generates
a "@":"@" entry into the aliases file; if this entry is
absent, sendmail assumes that the file is truncated. Perhaps
the convention on HP-UX is that the invalid entry consists
of an empty string pair.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-11-04 01:54
Message:
Logged In: YES
user_id=33168
How can I tell if NIS+ is being used? Martin do you have an
account on the Compaq testdrive machines?
The values are -1 coming in from yp_all as seen from the
stack trace:
#1 0x8f44c in PyString_FromStringAndSize (str=0x7f7f2e18
"\377\377\377\377",
size=-1) at Objects/stringobject.c:85
#2 0xc11ca5bc in nis_foreach (instatus=1,
inkey=0x7f7f2e18 "\377\377\377\377", inkeylen=-1,
inval=0x7f7f3220 "{\004\006P", invallen=-1,
indata=0x7f7f2698)
at /tmp/python/Modules/nismodule.c:95
#3 0xc02ff02c in xdr_ypall () from /usr/lib/libnsl.1
#4 0xc02daab4 in xdrrec_skiprecord () from /usr/lib/libnsl.1
#5 0xc02f88c8 in yp_all () from /usr/lib/libnsl.1
#6 0xc11cad68 in nis_cat (self=0x0, args=0x40c80a48)
at /tmp/python/Modules/nismodule.c:168
I don't see a specific problem from the man page. Here are
some relevant sections:
int yp_all(
char *indomain,
char *inmap,
struct ypall_callback *incallback
);
struct ypall_callback *incallback {
int (*foreach)();
char *data;
};
The function foreach() is called as follows:
foreach(
int instatus;
char *inkey;
int inkeylen;
char *inval;
int invallen;
char *indata;
);
instatus Holds one of the return status values defined in
<rpcsvc/yp_prot.h>: either YP_TRUE
or an error code (see ypprot_err()
below, for a function that converts
a NIS protocol error code to a
ypclnt layer error code, as defined
in <rpcsvc/ypclnt.h>).
inkey The key and value parameters are
inval somewhat different than defined in
the SYNOPSIS section above. First,
the memory pointed to by inkey and
inval is private to yp_all(), and
is overwritten with the arrival of
each new key-value pair.
Therefore, foreach() should do
something useful with the contents
of that memory, but it does not own
the memory. Key and value objects
presented to the foreach() look
exactly as they do in the server's
map. Therefore, if they were not
newline-terminated or null-
terminated in the map, they will
not be terminated with newline or
null characters here, either.
indata Is the contents of the
incallback->data element passed to
yp_all() The data element of the
callback structure can share state
information between foreach() and
the mainline code. Its use is
optional, and no part of the NIS
client package inspects its
contents. Cast it to something
useful or ignore it as appropriate.
The foreach() function is Boolean. It should
return zero to indicate it needs to be called
again for further received key-value pairs, or
non-zero to stop the flow of key-value pairs. If
foreach() returns a non-zero value, it is not
called again and the functional value of yp_all()
is then 0.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-11-04 00:11
Message:
Logged In: YES
user_id=21627
A quick test shows that indeed the if(fix) block causes the
trouble; it crashes with mail.aliases, because both strings
are empty.
I'm not entirely sure what the fix mechanism is supposed to
achieve; it does appear that it indeed avoids copying an
extra null byte on Solaris. The comment about "makedbm -a"
sounds mystical: makedbm has no documented -a option. We
should probably ask Fred Gansevles, who added this in 2.15.
There is also a GvR comment who says it doesn't work for NIS+.
Unless a better strategy shows up, I suggest to skip entries
which have both empty keys and values.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-11-03 23:32
Message:
Logged In: YES
user_id=21627
The patch looks wrong. What is the value of inkeylen and
invallen at the point of the crash? Might it be -1, due to
the prior decrement?
Was that for a 32-bit or a 64-bit binary? Could it be that
Python is using an incorrect signature of the foreach
function (despite the man page saying that this is the
correct signature)?
Could it be that the data are really large unsigned numbers?
If so, what are the corresponding data? The foreach function
is supposedly called once per record, so both sizes ought to
be small.
I am concerned about thread-safety of this entire module,
though. yp_all is invoked with the GIL released, yet the
callback function calls interpreter API. This asks for a
desaster if other threads simultanously access the interpreter.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=633013&group_id=5470