[comp.unix.programmer] System call error handling

chip@tct.uucp (Chip Salzenberg) (02/06/91)

[ Followups to comp.unix.programmer, since UNIX system call
  error handling is the current subject. ]

According to brnstnd@kramden.acf.nyu.edu (Dan Bernstein):
>Summary: Chip complains that a read() from disk into a ``newuid''
>integer may fail and hence have pty switch to the wrong uid. But I
>explicitly initialized newuid to uid just to handle this possibility.

That's fine for pty, unless the file is less than sizeof(int) bytes
long, in which case the initialization will not help.  Also, a disk
read error is worth reporting, no matter when it occurs, or to whom.

I would suggest, then, that pty should have code like this:

    if ((rd = read(fd, (char *)&newuid, sizeof(int))) != sizeof(int))
    {
        if (rd == -1)
            fprintf(stderr, "pty: can't read uid file %s: %s\n",
                    filename, strerror(errno));
        else
            fprintf(stderr, "pty: corrupt uid file %s\n", filename);
        newuid = uid;
    }

Yes, it's tedious.  But to omit tests for these possibilities when
there is something to be done is poor programming practice, with no
ifs, ands or buts.  (Printing an error message _is_ doing something.)

>Die with a fatal error, possibly killing a truly critical system program
>running under pty?

Death is irrelevant to the issue; it is also, in this case,
unnecessary.  Reporting unexpected system call errors is neither.

>Nobody's programming is perfect; the best I hope I can say is that I've
>gotten better over the years.

I would have to say the same thing about my own programming; so would
anyone with a modicum of honesty.  But the fact that one's programming
is better today than yesterday should -- I repeat, _should_ -- cause
one to be at least _polite_ when criticizing another's code.  The ego
you save may be your own.

>Don't you read code before you criticize it?

Context is always important.  But there are some things that you just
_don't_ mess around with, and user ids are at the top of the list.

>Chip, be reasonable. You can't demand of system programs that they check
>for external system consistency at every step.

Oh, really?  Read Deliver 2.0 PL12 and make that statement again.
-- 
Chip Salzenberg at Teltronics/TCT     <chip@tct.uucp>, <uunet!pdn!tct!chip>
 "Most of my code is written by myself.  That is why so little gets done."
                 -- Herman "HLLs will never fly" Rubin

brnstnd@kramden.acf.nyu.edu (Dan Bernstein) (02/06/91)

In article <27AEF248.6C25@tct.uucp> chip@tct.uucp (Chip Salzenberg) writes:
> I would suggest, then, that pty should have code like this:
  [ error reporting ]

But the process in question almost certainly doesn't have stderr! This
isn't just an artifact of some arbitrary design decision; if, for
example, stderr is pointing to the user's original terminal, then under
at least Clare's interpretation of POSIX ``security'' the process
shouldn't even be able to keep access to it. Furthermore, the master
process may become disconnected (as per Steve Bellovin's session manager
design) and reattach to a different process; what is it supposed to do
with stderr in this case? There are some solutions---like shuttling new
stderrs back and forth---but they introduce more unreliability than
there is in the original code you're complaining about.

> Yes, it's tedious.  But to omit tests for these possibilities when
> there is something to be done is poor programming practice, with no
> ifs, ands or buts.  (Printing an error message _is_ doing something.)

I'd probably agree with you if printing an error message were feasible
in that section of code. But it's not.

Again, could you please make the effort to read the code in context
before you criticize it?

> >Don't you read code before you criticize it?
> Context is always important.  But there are some things that you just
> _don't_ mess around with, and user ids are at the top of the list.

If anyone, anywhere, ever sees pty break security, I'll repent.

> >Chip, be reasonable. You can't demand of system programs that they check
> >for external system consistency at every step.
> Oh, really?  Read Deliver 2.0 PL12 and make that statement again.

Chip, be reasonable. You can't demand of system programs that they check
for external system consistency at every step.

---Dan

greywolf@unisoft.UUCP (The Grey Wolf) (02/07/91)

(Dan Bernstein) writes:
[ much deleted to cast a long story to a short one; ]
>>Die with a fatal error, possibly killing a truly critical system program
>>running under pty?

Dan, what in *hell* are you doing running a truly critical system program
under pty in the first place?  I mean, is it *really* necessary?

Truly critical system programs should be run from a real shell.

>
>>Chip, be reasonable. You can't demand of system programs that they check
>>for external system consistency at every step.

If you don't do this, you're setting yourself up for a MAJOR loss.
Moreover, if you're running Truly Critical System Programs that DON'T
check for some degree of sanity, you're just asking for trouble.  Most
system programs that I know of do sanity checks reasonably often.

(Kernels seem to have a problem with this on occasion :-), but that's
another story.)

-- 
thought:  I ain't so damb dumn!	| Your brand new kernel just dump core on you
war: Invalid argument		| And fsck can't find root inode 2
				| Don't worry -- be happy...
...!{ucbvax,acad,uunet,amdahl,pyramid}!unisoft!greywolf

brnstnd@kramden.acf.nyu.edu (Dan Bernstein) (02/07/91)

In article <3354@unisoft.UUCP> greywolf@unisoft.UUCP (The Grey Wolf) writes:
> (Dan Bernstein) writes:
> [ much deleted to cast a long story to a short one; ]
> >>Die with a fatal error, possibly killing a truly critical system program
> >>running under pty?
> Dan, what in *hell* are you doing running a truly critical system program
> under pty in the first place?  I mean, is it *really* necessary?
> Truly critical system programs should be run from a real shell.

Agreed. (Real shell? Do you have a real shell? I'd love to see the docs.)
Still, on the off chance that someone *is* running a critical program
under pty, it can't make a policy of dying at the first hint of trouble.

> >>Chip, be reasonable. You can't demand of system programs that they check
> >>for external system consistency at every step.
> If you don't do this, you're setting yourself up for a MAJOR loss.
> Moreover, if you're running Truly Critical System Programs that DON'T
> check for some degree of sanity, you're just asking for trouble.  Most
> system programs that I know of do sanity checks reasonably often.

Reasonably often, yes. (We are, after all, talking about a program that
checks nearly 100% of its return codes.) But defensive programming isn't
black and white. You can't demand that a program check before every
single operation that the entire rest of the program will succeed; the
system doesn't even provide hooks for simple operations like
preallocating filesystem space.

---Dan