Why is this not a qmail bug?

Question

I read DJB "Some thoughts on security after ten years of Qmail 1.0" and he listed this function to move the file descriptor:

 int fd_move (to, from)
 int to;
 int from;
 {
   if (to == from) return 0;
   if (fd_copy (to, from) == -1) return -1;
   close (from);
   return 0;
 }

It occurred to me that this code does not check the return value of close, so I read the man page to close (2), and it seems that it might fail with EINTR , in which case the corresponding behavior would look like to call again with that same argument.

Since this code was written by someone with much more experience than I did in both C and UNIX, and moreover, it hasn't changed in qmail for a decade, I guess there must be some kind of nuance which I am missing, which makes this code true. Can anyone explain this nuance to me?

Answer 1

When the file descriptor is duplicated, as in the fd_copy or dup2 function, you will get more than one file descriptor that refers to the same thing (the same as the struct file in the kernel). Closing one of them will simply decrease the reference count. The operation on the base object is not performed, if this is not the last close. As a result, conditions such as EINTR and EIO are not possible.

Answer 2

I have two answers:

• He tried to conclude that the code is factorized, and often such examples skip error checking for brevity and clarity.
• close (2) can return EINTER, but does it in practice, and if so, what would you reasonably do? Repeat once? To retry? What if you get EIO? That could mean almost everything, so you really have no reasonable means other than registration and moving. If you try again after EIO, you can get EBADF, then what? Suppose the handle is closed and moving on?

Each system call can return an EINTR, especially one that blocks reading (2), waiting for a slow person. This is a more likely scenario, and the good “get input from terminal” procedure will really check that. It also means that write (2) can fail even when writing a log file. Are you trying to log the error generated by the log, or you just have to refuse?

Answer 3

Another possibility is that its function is used only in the application (or part) that has done something to ensure that the call is not interrupted by a signal. If you are not going to do anything important with signals, then you do not need to respond to them, and it might make sense to mask them all, rather than wrapping each individual system call in an EINTR attempt. Except, of course, those who will kill you, so SIGKILL and often SIGPIPE, if you deal with this, quit smoking along with SIGSEGV and similar fatal errors, which in any case will never be delivered to the correct application for user space.

In any case, if all he talks about is security, then, quite possibly, he does not need to repeat close . If EIO does not work, then he will not be able to retry, it will be a permanent failure. Therefore, for the correctness of its program, close not required. It is possible that for the correctness of its program, it is not necessary to repeat the close attempt on EINTR.

Usually you want your program to do its best to succeed, which means you have to try again at EINTR. But this is a separate safety concern. If your program is designed in such a way that some function does not work for any reason, is not a security flaw, then, in particular, the fact that it did not accidentally fail EINTR, and not for a permanent reason, is not a flaw. It is known that the DJB is quite self-confident, so I would not be surprised if he proved why he does not need to try again, and therefore does not bother, even if this allows his program to succeed in resetting the knob in certain situations, where, perhaps, it is currently failing (for example, explicitly sending the user a harmless signal with kill at a crucial moment).

Edit: It seems to me that retrying EINTR could potentially be a security flaw under certain conditions. It introduces a new behavior into this section of code: it can loop endlessly in response to a signal stream, where earlier it would make one close attempt, and then return. I don’t know for sure that this will cause qmail any problems (after all, close itself does not guarantee how soon it will return). But if you give up one attempt, make the code easier to analyze, then this may be a reasonable step. Or not.

You might think that retrying prevents a DoS error when a signal causes a false failure. But retrying allows another (more complex) drawback of DoS when the signal stream causes an indefinite stall. In binary terms, “can this application be DoSed?”, What is the absolute security issue that DJB was interested in when qmail and djbdns wrote, it does not matter. If something can happen once, then usually it means that it can happen many times.

Answer 4

Only broken joins return an EINTR without an explicit request. The reasonable semantics for signal() allow you to restart system calls ("BSD style"). When creating a program on a system with sysv semantics (signal interruption), you should always replace calls to signal() with calls to bsd_signal() , which you can define yourself from the point of view of sigaction() , if it does not exist.

It should be further noted that no systems return an EINTR when a signal is received, if you have not installed signal handlers. If the default action remains in place or if the signal is not set, it is not possible to interrupt system calls.