[Patches] [ python-Patches-633013 ] Fix NIS causing interpreter core dump

noreply@sourceforge.net noreply@sourceforge.net
Mon, 04 Nov 2002 05:10:34 -0800


Patches item #633013, was opened at 2002-11-03 16: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: None
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: Neal Norwitz (nnorwitz)
Date: 2002-11-04 08: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 02: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-03 19: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-03 18: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 17: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