smtp.c+80: check for short write()

[Rolf Weber]

Hi!

While debugging another issue i ran across the code at https://bitbucket.org/jsuto/piler/src/538019632510868ffb9dc9324dc8fe1020a83baa/src/smtp.c#lines-80

There is no check for the amount actually written and the byte counter at session->tot_len is unconditionally increased by buflen.
However, there is no guarantee that write() actually has written buflen bytes. In fact, even returning 0 and having written 0 bytes would be permissible.
This would lead to (buflen - ret) missing bytes in the archived mail file. I think I saw this phenomenon a while back, when users complained about some broken mails in the archive.

[Janos SUTO]

Hello Rolf, thank you for reporting the issues. How about such fix? (Not tested yet). Perhaps process_data() should return with a result code, and the smtp connection might be closed in case of such scenario. Or instead of closing it immediately, rather wait until the end of the DATA phase, and provide a proper smtp error code.

‌

void process_data(struct smtp_session *session, char *buf, int buflen){
   int len=0, written=0;

   if(session->last_data_char == '\n' && strcmp(buf, ".\r\n") == 0){ 
      process_command_period(session);
   }   
   else {
      while(written < buflen) {
         len = write(session->fd, buf+written, buflen-written);
         if(len > 0){ 
            written += len;
            session->tot_len += len;
         }
         else syslog(LOG_PRIORITY, "ERROR (line: %d) process_data(): written %d bytes", __LINE__, len);
      }   
   }   

   session->last_data_char = buf[buflen-1];
}

However, I’m a bit confused, because the manual says

“The number of bytes written may be less than count if, for example, there is insufficient space on the underlying physical medium, or the RLIMIT_FSIZE resource limit is encountered (see setrlimit(2)), or the call was interrupted by a signal handler after having written less than count bytes. (See also pipe(7).”

Do you think either of the above conditions happened on your host? Also could you describe the host producing such errors? I mean the filesystem, free disk space, memory, etc. anything that can be relevant.

‌

[Rolf Weber]

Looks good, I’ve just rebuilt the binary and testing it now.
I’ll let you know if there are in fact some short writes.

The way I understood the man page for write() these are just some examples. I’m not aware of what the I/O-scheduler actually does, but I could imagine there are some provisions for block alignment and such.

[Janos SUTO]

Any update on the matter?

[Janos SUTO]

No news is good news.