[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110721170255.GA7352@albatros>
Date: Thu, 21 Jul 2011 21:02:55 +0400
From: Vasiliy Kulikov <segoon@...nwall.com>
To: musl@...ts.openwall.com
Subject: some fixes to musl
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 <unistd.h>
#include <pty.h>
#include <stdio.h>
+#include <errno.h>
/* 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 <signal.h>
#include <string.h>
#include <pthread.h>
+#include <errno.h>
#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);
--
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
