picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

fileio: fileno() can realistically return -1

Browse source 
An stdio FILE* stream usually refers to something with a file
descriptor, but that's just "usually". It doesn't have to, when taking
fmemopen() and similar into account. Most of our calls to fileno()
assumed the call couldn't fail. In most cases this was correct, but in
some cases where we didn't know whether we work on files or memory we'd
use the returned fd as if it was unconditionally valid while it wasn't,
and passed it to a multitude of kernel syscalls. Let's fix that, and do
something reasonably smart when encountering this case.

(Running test-fileio with this patch applied will remove tons of ioctl()
calls on -1).
This commit is contained in:
Lennart Poettering 2020-04-13 10:09:44 +02:00
parent 9d5dac4dce
commit 14f594b995
3 changed files with 56 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
src/basic/fileio.c
View file

@ -119,7 +119,7 @@ int write_string_stream_ts(
struct timespec *ts) {
bool needs_nl;
int r;
int r, fd;
assert(f);
assert(line);
@ -127,6 +127,14 @@ int write_string_stream_ts(
if (ferror(f))
return -EIO;
if (ts) {
/* If we shall set the timestamp we need the fd. But fmemopen() streams generally don't have
* an fd. Let's fail early in that case. */
fd = fileno(f);
if (fd < 0)
return -EBADF;
}
needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n");
if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) {
@ -154,7 +162,7 @@ int write_string_stream_ts(
if (ts) {
struct timespec twice[2] = {*ts, *ts};
if (futimens(fileno(f), twice) < 0)
if (futimens(fd, twice) < 0)
return -errno;
}
@ -886,7 +894,7 @@ int fflush_and_check(FILE *f) {
}
int fflush_sync_and_check(FILE *f) {
int r;
int r, fd;
assert(f);
@ -894,10 +902,16 @@ int fflush_sync_and_check(FILE *f) {
if (r < 0)
return r;
if (fsync(fileno(f)) < 0)
/* Not all file streams have an fd associated (think: fmemopen()), let's handle this gracefully and
* assume that in that case we need no explicit syncing */
fd = fileno(f);
if (fd < 0)
return 0;
if (fsync(fd) < 0)
return -errno;
r = fsync_directory_of_file(fileno(f));
r = fsync_directory_of_file(fd);
if (r < 0)
return r;
@ -1074,8 +1088,16 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
* \n as the single EOL marker, so there is no need to wait. We check
* this condition last to avoid isatty() check if not necessary. */
if (tty < 0)
tty = isatty(fileno(f));
if (tty < 0) {
int fd;
fd = fileno(f);
if (fd < 0) /* Maybe an fmemopen() stream? Handle this gracefully,
* and don't call isatty() on an invalid fd */
tty = false;
else
tty = isatty(fd);
}
if (tty > 0)
break;
}

32
src/basic/terminal-util.c
View file

@ -81,31 +81,34 @@ int chvt(int vt) {
int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
_cleanup_free_ char *line = NULL;
struct termios old_termios;
int r;
int r, fd;
assert(f);
assert(ret);
/* If this is a terminal, then switch canonical mode off, so that we can read a single character */
if (tcgetattr(fileno(f), &old_termios) >= 0) {
/* If this is a terminal, then switch canonical mode off, so that we can read a single
* character. (Note that fmemopen() streams do not have an fd associated with them, let's handle that
* nicely.) */
fd = fileno(f);
if (fd >= 0 && tcgetattr(fd, &old_termios) >= 0) {
struct termios new_termios = old_termios;
new_termios.c_lflag &= ~ICANON;
new_termios.c_cc[VMIN] = 1;
new_termios.c_cc[VTIME] = 0;
if (tcsetattr(fileno(f), TCSADRAIN, &new_termios) >= 0) {
if (tcsetattr(fd, TCSADRAIN, &new_termios) >= 0) {
char c;
if (t != USEC_INFINITY) {
if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0) {
(void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
if (fd_wait_for_event(fd, POLLIN, t) <= 0) {
(void) tcsetattr(fd, TCSADRAIN, &old_termios);
return -ETIMEDOUT;
}
}
r = safe_fgetc(f, &c);
(void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
(void) tcsetattr(fd, TCSADRAIN, &old_termios);
if (r < 0)
return r;
if (r == 0)
@ -119,8 +122,13 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
}
}
if (t != USEC_INFINITY) {
if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0)
if (t != USEC_INFINITY && fd > 0) {
/* Let's wait the specified amount of time for input. When we have no fd we skip this, under
* the assumption that this is an fmemopen() stream or so where waiting doesn't make sense
* anyway, as the data is either already in the stream or cannot possible be placed there
* while we access the stream */
if (fd_wait_for_event(fd, POLLIN, t) <= 0)
return -ETIMEDOUT;
}
@ -778,6 +786,9 @@ const char *default_term_for_tty(const char *tty) {
int fd_columns(int fd) {
struct winsize ws = {};
if (fd < 0)
return -EBADF;
if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
return -errno;
@ -812,6 +823,9 @@ unsigned columns(void) {
int fd_lines(int fd) {
struct winsize ws = {};
if (fd < 0)
return -EBADF;
if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
return -errno;

6
src/shared/conf-parser.c
View file

@ -294,7 +294,7 @@ int config_parse(const char *unit,
_cleanup_fclose_ FILE *ours = NULL;
unsigned line = 0, section_line = 0;
bool section_ignored = false, bom_seen = false;
int r;
int r, fd;
assert(filename);
assert(lookup);
@ -311,7 +311,9 @@ int config_parse(const char *unit,
}
}
fd_warn_permissions(filename, fileno(f));
fd = fileno(f);
if (fd >= 0) /* stream might not have an fd, let's be careful hence */
fd_warn_permissions(filename, fd);
for (;;) {
_cleanup_free_ char *buf = NULL;