[net.bugs.4bsd] crypt can get confused when in pipes

richl@daemon.UUCP (Rick Lindsley) (01/23/85)

Index:	/usr/src/usr.bin/crypt.c 4.2BSD

Description:
	When crypt was included in a complex arrangement of pipes and
	processes, it failed to work, saying "crypt: cannot generate key".
	Of course it worked fine when alone.

Repeat-By:
	Here are the contents of a shell script "shscript". I've simplified
	it somewhat to the point where the bug is still apparent, so don't
	look at it and say "but WHY would anyone want to do that??"

	#! /bin/sh
	case $1 in
	-e) cat | crypt ;;

	-d) cat | cat | crypt ;;
	esac
	exit 0

	---
	(What's interesting is that the two cats are needed to demonstrate
	this bug.)

	% echo JJJ DDD ooo | crypt > test.out
	Enter key:foo
	% cat test.out | shscript -d
	Enter key:foo
	crypt: cannot generate key.
	%

Fix:
    Crypt was writing to a pipe that it was using as a full duplex
    communication channel to another process. This is kind of unreliable,
    because if the timing is wrong one can read their own data back out of
    the pipe before the intended process can receive it. Crypt got around
    this by doing a wait() on the receiving process to wait for it to die
    (thus it MUST have read the data) but it never checked the pid returned
    -- and when something else in a complicated pipe arrangement exited first
    the above scenario took place.

    I'm not sure why crypt inherited another process in the first place...I
    think something else may smell a bit here, too, especially when one
    considers the contrivance one must go through to make this bug apparent.

    The elegant fix would be to have crypt use two pipes, one each for
    stdin and stdout and use them as half-duplex communication lines. But,
    another option which takes less effort is to have crypt wait() in a
    loop, checking the pid it got against the one it was looking for. That
    way it REALLY knows when makekey is dead. The latter choice is the one
    I implemented, by virtue of the fact that it is easier.

    Line numbers will doubtless be different due to headers, but it shouldn't
    be too confusing.

    22a29
    > 	int pid,wpid;
    32c39
    < 	if (fork()==0) {
    ---
    > 	if ((pid=fork())==0) {
    42c49,50
    < 	wait((int *)NULL);
    ---
    > 	while ((wpid = wait((int *)NULL)) != -1 && wpid != pid)
    > 	    ;

Rick Lindsley
richl@tektronix.csnet
...!{decvax,ihnp4,allegra,ucbvax}!tektronix!richl

henry@utzoo.UUCP (Henry Spencer) (01/30/85)

>    I'm not sure why crypt inherited another process in the first place...I
>    think something else may smell a bit here, too, especially when one
>    considers the contrivance one must go through to make this bug apparent.
>    [Running crypt at the end of at least a 3-stage pipeline.]

A well-known fact of life, which some programs don't pay quite enough
attention to, is that the earlier processes in complex sh pipelines are
the children of the later ones.  Processes doing a wait() should *always*
check to make sure that the child they end up with is the right one.
-- 
				Henry Spencer @ U of Toronto Zoology
				{allegra,ihnp4,linus,decvax}!utzoo!henry