From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/183 Path: news.gmane.org!not-for-mail From: Vasiliy Kulikov Newsgroups: gmane.linux.lib.musl.general Subject: some fixes to musl Date: Thu, 21 Jul 2011 21:02:55 +0400 Message-ID: <20110721170255.GA7352@albatros> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1311268080 10588 80.91.229.12 (21 Jul 2011 17:08:00 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Thu, 21 Jul 2011 17:08:00 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-267-gllmg-musl=m.gmane.org@lists.openwall.com Thu Jul 21 19:07:56 2011 Return-path: Envelope-to: gllmg-musl@lo.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by lo.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Qjwj1-00088T-4F for gllmg-musl@lo.gmane.org; Thu, 21 Jul 2011 19:07:51 +0200 Original-Received: (qmail 24317 invoked by uid 550); 21 Jul 2011 17:07:49 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 21760 invoked from network); 21 Jul 2011 17:03:53 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:date:from:to:subject:message-id:mime-version:content-type :content-disposition:user-agent; bh=2ipdhrLvpjzOeEkE1zhaoj0J3lSSCdcA7MuqNoqFz+k=; b=hKOClilQUfWjy7zSwvbn+qSWmMyC2fItkMGYPvQs9XIAkm17its9cT4/vV2jL4v+W3 4s3HItJ9s52ud1F48iqnZFGtWBDtnu7mBQ5UfZB3BuMwdMZTazXzyfSw897ObS+5Ob6S D4nCcuNpd6oIE/DnbTGFy2vDfLcm6OJSWRoPk= Original-Sender: Vasiliy Kulikov Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Xref: news.gmane.org gmane.linux.lib.musl.general:183 Archived-At: Hi, This is a patch against v0.7.12. It is only compile tested. fdopendir(): - POSIX defines EBADF if fd is invalid. I suppose it is OK to assume fd is invalid if fstat() failed. - fcntl(fd, F_SETFD, FD_CLOEXEC) may fail (at least in theory). If it fails, the consequences are not expected. rewinddir(), seekdir(): - lseek(fd, 0, SEEK_SET) may fail too, I suppose it can be true for network file systems or FUSE fses. Continuing the work with pos=-1 is somewhat not expected. cuserid(): - POSIX says an argument may be NULL, in this case the buffer should be statically allocated. forkpty(): - It should be guaranteed that master fd is closed, tty is setup, slave fd is dup'ed to 1,2,3. The latter can be broken by setting small rlimit. setsid() is checked for company :) I think the only way to handle the failure is _exit(). While it may be not the best choise, however, continuing the work with half dropped privileges is more dangerous. openpty(): - close() shouldn't change errno updated by failed ioctl()/open(). - I suppose the last calls to tcsetattr() and ioctl() may fail too. realpath() (no patch): - IMO the logic is wrong. While opening the file and asking the kernel the full path is interesting, it won't work with not readable files. Also if a file is a device or tty the opening might have unexpected side effects. I don't dare to reimplement it myself, though ;-) _vsyslog(): - sendto() may fail in case of signal delivery. I wonder what is the Right Way to handle close() failures in generic case. If it is fork'ish function, _exit() can be used. If it is some privilege dropping function, failure can be returned in errno, but the task state (hanged/unclosed fd) is not expected by the caller. More dangerous case is function that is not designed to return error at all. E.g. close() inside of closelog() may fail, but the caller cannot be notified about it by design. If the caller want to do chroot(), which prevents re-opening log fd, the failure would break task's expectation of dropped privileges (closed log fd). diff --git a/src/dirent/fdopendir.c b/src/dirent/fdopendir.c index c4b8e61..0aaa735 100644 --- a/src/dirent/fdopendir.c +++ b/src/dirent/fdopendir.c @@ -12,7 +12,11 @@ DIR *fdopendir(int fd) DIR *dir; struct stat st; - if (fstat(fd, &st) < 0 || !S_ISDIR(st.st_mode)) { + if (fstat(fd, &st) < 0) { + errno = EBADF; + return 0; + } + if (!S_ISDIR(st.st_mode)) { errno = ENOTDIR; return 0; } @@ -20,7 +24,10 @@ DIR *fdopendir(int fd) return 0; } - fcntl(fd, F_SETFD, FD_CLOEXEC); + if (fcntl(fd, F_SETFD, FD_CLOEXEC)) { + free(dir); + return 0; + } dir->fd = fd; return dir; } diff --git a/src/dirent/rewinddir.c b/src/dirent/rewinddir.c index c6138f7..afa80ac 100644 --- a/src/dirent/rewinddir.c +++ b/src/dirent/rewinddir.c @@ -6,8 +6,9 @@ void rewinddir(DIR *dir) { LOCK(&dir->lock); - lseek(dir->fd, 0, SEEK_SET); - dir->buf_pos = dir->buf_end = 0; - dir->tell = 0; + if (lseek(dir->fd, 0, SEEK_SET) != (off_t)-1) { + dir->buf_pos = dir->buf_end = 0; + dir->tell = 0; + } UNLOCK(&dir->lock); } diff --git a/src/dirent/seekdir.c b/src/dirent/seekdir.c index 81a0e33..df4f403 100644 --- a/src/dirent/seekdir.c +++ b/src/dirent/seekdir.c @@ -5,8 +5,13 @@ void seekdir(DIR *dir, long off) { + off_t pos; + LOCK(&dir->lock); - dir->tell = lseek(dir->fd, off, SEEK_SET); - dir->buf_pos = dir->buf_end = 0; + pos = lseek(dir->fd, off, SEEK_SET); + if (pos != (off_t)-1) { + dir->tell = pos; + dir->buf_pos = dir->buf_end = 0; + } UNLOCK(&dir->lock); } diff --git a/src/misc/cuserid.c b/src/misc/cuserid.c index 4e78798..4bf9450 100644 --- a/src/misc/cuserid.c +++ b/src/misc/cuserid.c @@ -7,6 +7,10 @@ char *cuserid(char *buf) { struct passwd pw, *ppw; long pwb[256]; + static char cbuff[L_cuserid]; + + if (buf == NULL) + buf = cbuff; if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw)) return 0; snprintf(buf, L_cuserid, "%s", pw.pw_name); diff --git a/src/misc/forkpty.c b/src/misc/forkpty.c index 0bbf2de..2fc0dec 100644 --- a/src/misc/forkpty.c +++ b/src/misc/forkpty.c @@ -10,12 +10,13 @@ int forkpty(int *m, char *name, const struct termios *tio, const struct winsize if (openpty(m, &s, name, tio, ws) < 0) return -1; pid = fork(); if (!pid) { - close(*m); - setsid(); - ioctl(s, TIOCSCTTY, (char *)0); - dup2(s, 0); - dup2(s, 1); - dup2(s, 2); + if (close(*m) || + (setsid() == (pid_t)-1) || + (ioctl(s, TIOCSCTTY, (char *)0) == -1) || + (dup2(s, 0) == -1) || + (dup2(s, 1) == -1) || + (dup2(s, 2) == -1)) + _exit(1); if (s>2) close(s); return 0; } diff --git a/src/misc/openpty.c b/src/misc/openpty.c index 0b4eb22..14e4329 100644 --- a/src/misc/openpty.c +++ b/src/misc/openpty.c @@ -3,6 +3,7 @@ #include #include #include +#include /* Nonstandard, but vastly superior to the standard functions */ @@ -10,24 +11,41 @@ int openpty(int *m, int *s, char *name, const struct termios *tio, const struct { int n=0; char buf[20]; + int old_errno; *m = open("/dev/ptmx", O_RDWR|O_NOCTTY); if (!*m) return -1; if (ioctl(*m, TIOCSPTLCK, &n) || ioctl (*m, TIOCGPTN, &n)) { + old_errno = errno; close(*m); + errno = old_errno; return -1; } if (!name) name = buf; snprintf(name, sizeof buf, "/dev/pts/%d", n); if ((*s = open(name, O_RDWR|O_NOCTTY)) < 0) { + old_errno = errno; close(*m); + errno = old_errno; return -1; } - if (tio) tcsetattr(*s, TCSANOW, tio); - if (ws) ioctl(*s, TIOCSWINSZ, ws); + if (tio && tcsetattr(*s, TCSANOW, tio)) { + old_errno = errno; + close(*s); + close(*m); + errno = old_errno; + return -1; + } + if (ws && ioctl(*s, TIOCSWINSZ, ws)) { + old_errno = errno; + close(*s); + close(*m); + errno = old_errno; + return -1; + } return 0; } diff --git a/src/misc/syslog.c b/src/misc/syslog.c index cbe6520..5cd5a09 100644 --- a/src/misc/syslog.c +++ b/src/misc/syslog.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "libc.h" static int lock; @@ -73,6 +74,8 @@ static void _vsyslog(int priority, const char *message, va_list ap) char buf[256]; int pid; int l, l2; + ssize_t sent; + int pos; if (log_fd < 0) { __openlog(log_ident, log_opt | LOG_NDELAY, log_facility); @@ -95,7 +98,17 @@ static void _vsyslog(int priority, const char *message, va_list ap) if (l2 >= 0) { l += l2; if (buf[l-1] != '\n') buf[l++] = '\n'; - sendto(log_fd, buf, l, 0, (void *)&log_addr, 11); + pos = 0; + while (l) { + sent = sendto(log_fd, buf+pos, l, 0, (void *)&log_addr, 11); + if (sent == -1) { + if (errno == EINTR || errno == EAGAIN) + continue; + break; + } + l -= sent; + pos += sent; + } } UNLOCK(&lock); --