Bug 7524 - ppp-watch modifying ifcfg-ppp, and other bugs
Summary: ppp-watch modifying ifcfg-ppp, and other bugs
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Red Hat Linux
Classification: Retired
Component: initscripts
Version: 6.1
Hardware: i386
OS: Linux
medium
high
Target Milestone: ---
Assignee: Michael K. Johnson
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 1999-12-02 22:53 UTC by Adam Jenkins
Modified: 2008-05-01 15:37 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 1999-12-06 15:51:11 UTC
Embargoed:


Attachments (Terms of Use)

Description Adam Jenkins 1999-12-02 22:53:21 UTC
I've found a number of bugs in ppp-watch, from initscripts-4.70-1 (which is
the latest version I could find on update.redhat.com).

1)  When pppd is run via ppp-watch, the stdout from pppd (I have the "debug
connection" option on) gets appended to
/etc/sysconfig/network-scripts/ifcfg-ppp0.  Needless to say, this causes
problems the next time I try to connect and the connect script sources
ifcfg-ppp0.  I know this sounds bizarre, but it's true and I even have a
fix for it, described below.

2) The detach() function doesn't really detach.  This means that ifup, and
therefore usernetctl, don't go into the background after they're called,
like they used to.

3) ppp-watch and ifdown-ppp have different ideas of what the
/var/run/pppwatch* file will be called.  For example, if I connect using
the ppp0-netway interface, ppp-watch will create
/var/run/pppwatch-ppp0-netway.pid, but ifdown-ppp will look for
/var/run/pppwatch-ppp0.pid.  That is, ifdown-ppp just looks for the real
interface name, and ignores the clone name, but pppwatch doesn't.
Therefore ifdown no longer works.

4) Another reason ifdown doesn't work, is that when ppp-watch receives a
SIGTERM, it doesn't correctly kill pppd.  This is because the
pppLogicalToPhysical() function is completely bogus.  First of all, again
it doesn't strip away the clone name; if the device name is ppp0-netway, it
doesn't strip away the "-netway" part, so it looks for
/var/run/ppp-ppp0-netway.pid.  But if that weren't bad enough, it also
looks like it expects the interface name to stored in ppp-ppp0.pid, in
addition to the PID.  But pppd only writes the pid to that file.

There are probably other bugs, but by the time I found this many, I gave up
on it.

Since bug 1) is the wierdest, I saved the description until last.  Here's
why that bug occurs, and how to fix it.

The detach() function is called from main, and forks.  The parent process
never returns from detach().  The child process, closes stdin, stdout, and
stderr, and then returns.  This means that the next file that ppp-watch
opens will get fd 0, and the next will get fd 1, etc.  Another fact; when
you fork, the child process inherits the stdin, stdout and stderr of the
parent process.  Well, ppp-watch calls shvarfilesGet(), which opens some
files.  The second file it opens is
/etc/sysconfig/network-scripts/ifcfg-ppp0, which gets file descriptor 1.
Then it forks and execs ifup-ppp.   This means that any stdoutput from
ifup-ppp or its child process gets appended to ifcfg-ppp0!

The fix is to patch fork_exec() so that it closes stdout and stderr before
calling exec.

Adam

Comment 1 Adam Jenkins 1999-12-03 07:42:59 UTC
I've made a patch to fix the above problems.  Actually one of the things I said
above wasn't correct.  pppd does write the real device name to in the
/var/run/ppp-<linkname>.pid file, it just doesn't put it there until a
connection has actually been made.

Here's my patch.  It fixes the following problems with initscripts-4.70-1.

1) Fixes the problem where child processes of ppp-watch would write to files
that ppp-watch had open.  In my case, the debug output of pppd was getting
appended to /etc/sysconfig/network-scripts/ifcfg-ppp0.

2) pppd couldn't do correct PAP authentication because of bugs in ifup-ppp.
Specifically, ifup-ppp was using the "name" option to pppd tell pppd the
username to use for authentication.  The problem with this is that pppd also
uses the "name" option to match the "server" field in the pap-secrets file when
looking for which secret to use.  My pap-secret line looks like this:

	ajenkins ppp0-netway mypassword

ifup-ppp was passing "name ajenkins" to pppd, so pppd would look for a line with
"ajenkins" in the second field, not find it, and fail to authenticate.  The fix
is to use the "user" option instead of "name".  Also, ifup-ppp was passing
"remotename $DEVICE" to pppd, when it should have been using "remotename
$DEVNAME", since pppd will use the remotename option to find which pap-secret
line to use.

3) pppd was storing its pid and interface info in /var/run/ppp-ppp0.pid, but
ppp-watch was looking for it in ppp-ppp0-netway.pid.  The fix was to make
ifup-ppp pass "linkname $DEVNAME" to pppd instead of "linkname $DEVICE".  This
causes pppd to use the same name that ppp-watch expects.

4) ppp-watch as storing its pid in /var/run/pppwatch-ppp0-netway.pid, but
ifdown-ppp was looking for it in /var/run/pppwatch-ppp0.pid.  The fix here was
to make ifdown-ppp use $DEVNAME instead of $DEVICE to generate the pid filename.

Here's a patch file against initscripts-4.70-1 that fixes the above bugs.

Adam

Only in initscripts-4.70/src: TAGS
diff -rc initscripts-4.70.orig/src/ppp-watch.c initscripts-4.70/src/ppp-watch.c
*** initscripts-4.70.orig/src/ppp-watch.c	Mon Nov 22 15:57:49 1999
--- initscripts-4.70/src/ppp-watch.c	Fri Dec  3 00:59:23 1999
***************
*** 231,236 ****
--- 231,240 ----
  	    setpgid(0, 0);
  	}

+ 	/* make sure child process can't overwrite our parent's files */
+ 	close(1);
+ 	close(2);
+
  	execl(path, path, arg1, arg2, arg3, NULL);
  	perror(path);
  	_exit (1);
***************
*** 335,350 ****

  static int
  interfaceStatus(char *device) {
!     int sock = 0;
      int pfs[] = {AF_INET, AF_IPX, AF_AX25, AF_APPLETALK, 0};
      int p = 0;
      struct ifreq ifr;
      int retcode = 0;

!     while (!sock && pfs[p]) {
          sock = socket(pfs[p++], SOCK_DGRAM, 0);
      }
!     if (!sock) return 0;

      memset(&ifr, 0, sizeof(ifr));
      strncpy(ifr.ifr_name, device, IFNAMSIZ);
--- 339,354 ----

  static int
  interfaceStatus(char *device) {
!   int sock = -1;
      int pfs[] = {AF_INET, AF_IPX, AF_AX25, AF_APPLETALK, 0};
      int p = 0;
      struct ifreq ifr;
      int retcode = 0;

!     while (sock < 0 && pfs[p]) {
          sock = socket(pfs[p++], SOCK_DGRAM, 0);
      }
!     if (sock < 0) return 0;

      memset(&ifr, 0, sizeof(ifr));
      strncpy(ifr.ifr_name, device, IFNAMSIZ);
diff -rc initscripts-4.70.orig/sysconfig/network-scripts/ifdown-ppp
initscripts-4.70/sysconfig/network-scripts/ifdown-ppp
*** initscripts-4.70.orig/sysconfig/network-scripts/ifdown-ppp	Wed Sep  8
16:14:38 1999
--- initscripts-4.70/sysconfig/network-scripts/ifdown-ppp	Fri Dec  3 01:20:43
1999
***************
*** 7,13 ****
  CONFIG=$1
  source_config

! file=/var/run/pppwatch-$DEVICE.pid

  if [ ! -f $file ]; then
      # ppp isn't running, or we didn't start it
--- 7,13 ----
  CONFIG=$1
  source_config

! file=/var/run/pppwatch-$DEVNAME.pid

  if [ ! -f $file ]; then
      # ppp isn't running, or we didn't start it
diff -rc initscripts-4.70.orig/sysconfig/network-scripts/ifup-ppp
initscripts-4.70/sysconfig/network-scripts/ifup-ppp
*** initscripts-4.70.orig/sysconfig/network-scripts/ifup-ppp	Fri Nov 19 15:57:53
1999
--- initscripts-4.70/sysconfig/network-scripts/ifup-ppp	Fri Dec  3 01:23:36 1999
***************
*** 74,80 ****
    opts="$opts ${IPADDR}:${REMIP}"
  fi
  if [ -n "${PAPNAME}" ] ; then
!   opts="$opts name ${PAPNAME}"
  fi
  if [ "${DEBUG}" = yes ] ; then
    opts="$opts debug"
--- 74,80 ----
    opts="$opts ${IPADDR}:${REMIP}"
  fi
  if [ -n "${PAPNAME}" ] ; then
!   opts="$opts user ${PAPNAME}"
  fi
  if [ "${DEBUG}" = yes ] ; then
    opts="$opts debug"
***************
*** 95,109 ****

  if [ -n "$WVDIALSECT" ] ; then
    exec /usr/sbin/pppd -detach $opts $MODEMPORT $LINESPEED \
!     remotename $DEVICE ipparam $DEVICE \
!     linkname $DEVICE \
      noauth \
      ${PPPOPTIONS} \
      connect "/usr/bin/wvdial --remotename $DEVICE --chat $WVDIALSECT"
  else
    exec /usr/sbin/pppd -detach $opts $MODEMPORT $LINESPEED \
!     remotename $DEVICE ipparam $DEVICE \
!     linkname $DEVICE \
      noauth \
      ${PPPOPTIONS} \
      connect "/usr/sbin/chat $chatdbg -f $CHATSCRIPT"
--- 95,109 ----

  if [ -n "$WVDIALSECT" ] ; then
    exec /usr/sbin/pppd -detach $opts $MODEMPORT $LINESPEED \
!     remotename $DEVNAME ipparam $DEVICE \
!     linkname $DEVNAME \
      noauth \
      ${PPPOPTIONS} \
      connect "/usr/bin/wvdial --remotename $DEVICE --chat $WVDIALSECT"
  else
    exec /usr/sbin/pppd -detach $opts $MODEMPORT $LINESPEED \
!     remotename $DEVNAME ipparam $DEVICE \
!     linkname $DEVNAME \
      noauth \
      ${PPPOPTIONS} \
      connect "/usr/sbin/chat $chatdbg -f $CHATSCRIPT"

Comment 2 Michael K. Johnson 1999-12-03 21:18:59 UTC
1) I've fixed in a different way -- I simply close the ifcfg file
descriptors after reading, before forking.  Note that this problem
can only occur with clone devices; otherwise, only fd 0 is open
and fd 1 isn't open and so the writes go nowhere.

3 and 4) Actually, the fix is the other way -- only one of a set
of clones can be active, and "ifdown ppp0" should take down the
ppp0 device brought up via "ifup ppp0-myclone", and all the locking
should be per device, not per clone.  Also fixed in my current
development sources.

2) I'm having trouble believing that PAP simply doesn't work when
I've tested it repeatedly and verified that PAP was indeed being
used, and when I've repeatedly heard from third parties that PAP
is working for them.  A quick perusal of the pppd man page indicates
that I need to spend some more time figuring out the right thing
to do to make PAP and CHAP both work correctly for everyone.

You are right that remotename should be set to DEVNAME instead
of DEVICE.

detach() works exactly as intended.  It was a bug (but a hard one
to fix correctly) that it didn't wait in the past; every other
network interface was brought up and down sychronously, and we
have just extended that to PPP.  Use & if you want it to happen
asynchronously...

Thanks for the detailed bug report and patch!

Comment 3 Michael K. Johnson 1999-12-03 21:40:59 UTC
OK, I think you are right.  How about this:

if [ -n "${PAPNAME}" ] ; then
  opts="$opts user ${PAPNAME} remotename ${DEVNAME}"
fi

(then remove the remotename stuff in the command line later)

Want to try that?  I was of the opinion that the remotename $DEVNAME
would override the lookup part of name ${PAPNAME}, but I guess not.
I don't understand why the PAP management code can work for most
people, but not for some.  :-(

------- Email Received From  "Adam P. Jenkins" <adampjenkins> 12/04/99 02:56 -------

Comment 4 Michael K. Johnson 1999-12-06 15:51:59 UTC
In email, Adam replies
>It works fine for me if I just use "user $PAPNAME" and "remotename
>$DEVNAME".
>...

It works for me in my testing also.  This will go into some RawHide
release at least, and probably into an errata release soon as well.
I want to test some of these changes (along with other changes) in
RawHide before making an errata release.  That's why, despite the
fact that these fixes will probably make it into an errata update
in the long term, I've marked the resolution "RawHide".

FWIW, I've heard of more failures with CHAP than with PAP, and I'm
particularly interested in finding out whether this resolves the
CHAP problems.

Thanks for the very detailed and useful bug report!

Comment 5 Michael K. Johnson 1999-12-06 20:58:59 UTC
The next RawHide, out probably today or tomorrow, will have
initscripts-4.71-1 or later, which will have these fixes.


Note You need to log in before you can comment on or make changes to this bug.