Bug 7318

An xdm child process crashes with SIGSEGV after I log in via XDMCP on The Hurd,
resulting in the greeter dialog box coming back up shortly after.  This gcc
warning tipped me as to the nature of the problem:

if gcc-4.1 -DHAVE_CONFIG_H -I. -I. -I.    -I/usr/local/include  
-I/usr/local/include   -I/usr/local/include   -I/usr/local/include  
-I/usr/local/include   -D_XOPEN_SOURCE -I/usr/local/include   -D_BSD_SOURCE
-DHASXDMAUTH -DSECURE_RPC  -march=pentium2 -g -MT xdm-session.o -MD -MP -MF
".deps/xdm-session.Tpo" -c -o xdm-session.o `test -f 'session.c' || echo
'./'`session.c; \
        then mv -f ".deps/xdm-session.Tpo" ".deps/xdm-session.Po"; else rm -f
".deps/xdm-session.Tpo"; exit 1; fi
session.c:160: warning: initialization from incompatible pointer type
session.c:162: warning: initialization from incompatible pointer type
session.c:164: warning: excess elements in struct initializer
session.c:164: warning: (near initialization for 'dlfuncs')

I observed the crash would occur just after executing greeter/verify.c:399.

---8<---8<---
Breakpoint 1, Verify (d=0x8068610, greet=0x8060c90, verify=0x8060ca8)
    at verify.c:399
399             pam_handle_t **pamhp = thepamhp();
(gdb) step
crypt (key=0x180 <Address 0x180 out of bounds>, 
    salt=0x120 <Address 0x120 out of bounds>) at crypt-entry.c:127
127     crypt-entry.c: No such file or directory.
        in crypt-entry.c
(gdb) up
#1  0x01425181 in Verify (d=0x8068610, greet=0x8060c90, verify=0x8060ca8)
    at verify.c:399
399             pam_handle_t **pamhp = thepamhp();
(gdb) print pamhp
$1 = (pam_handle_t **) 0x2b252b25
(gdb) print *pamhp
Cannot access memory at address 0x2b252b25
(gdb) bt
#0  crypt (key=0x180 <Address 0x180 out of bounds>, 
    salt=0x120 <Address 0x120 out of bounds>) at crypt-entry.c:127
#1  0x01425181 in Verify (d=0x8068610, greet=0x8060c90, verify=0x8060ca8)
    at verify.c:399
#2  0x01424c62 in GreetUser (d=0x8068610, dpy=0x101a7d8, verify=0x8060ca8, 
    greet=0x8060c90, dlfuncs=0x8060a40) at greet.c:439
#3  0x08053822 in ManageSession (d=0x8068610) at session.c:320
#4  0x0805101c in StartDisplay (d=0x8068610) at dm.c:687
#5  0x080583c7 in manage (from=0x101a8ac, fromlen=16, length=23, fd=5)
    at xdmcp.c:1290
#6  0x080569da in ProcessRequestSocket (fd=5) at xdmcp.c:399
#7  0x08055a55 in ProcessListenSockets (readmask=0x101a98c) at socket.c:528
#8  0x08056b51 in WaitForSomething () at xdmcp.c:447
#9  0x0804fffa in main (argc=6, argv=0x101ae54) at dm.c:212
--->8--->8---

I tracked it down to being caused by incongruity between struct dlfuncs and code
that initializes a variable of that type.  The problem is that there are several
#ifdefs and such littered in the body of that struct definition in greet.h. 
Code initializing this struct in session.c has similar, but not identical
preprocessor directives.

On the Hurd, the incongruity manifests itself as greet.h including this in
struct dlfuncs' definition:
...
#ifdef linux
    void (*_endpwent)(void);
#endif
...

And session.c initializing the (struct dlfuncs) dlfuncs variable with this
corresponding code:
...
#if defined(linux) || defined(__GLIBC__)
        endpwent,
#endif
...

On Linux systems, both conditions are true, but only the latter is true on The
Hurd.  This means the following member of struct dlfuncs, _crypt, is assigned
the pointer to endpwent.  There's a smattering of function stuff relating to
dlfuncs which also required correction to fix this problem--the true problem
probably lied there.  I can't say I really understand all of what's going on
with dlfuncs and the related code, other than that the preprocessor directives
apparently must be identical.

This problem also may affect QNX, because of incongruity relating to it in the
same fashion.

Attached is a patch against xdm, checked out of CVS a short while ago, to fix
this problem.  I've assumed that the pattern of preprocessor directives in
session.c should be considered authoritative, and have ensured that similar
patterns throughout the code match that one.

Although I had problems building xdm from CVS, probably due to my build
environment and the autotools, the patch applies to xdm-X11R7.1-1.0.4 and
results in a correct build[1].

If I might make a humble suggestion, you may want to add a comment above every
part of code that has such a pattern (as identified by which code my patch
touches) in order to advise future developers to copy any changes they make to
the preprocessor directives to the other places that need those changes.  Better
yet, a less fragile refactoring where the preprocessor directives are only used
once would be exponentially better.

-----

1. As long as --disable-secure-rpc is passed to configure, and the workaround
described in bug #7317 is applied.