Discussion:
[PATCH] af_alg: recover better from crypto failures
Paul Eggert
2018-05-10 01:10:08 UTC
Permalink
* lib/af_alg.c (afalg_stream): Recover from crypto failures if the
input stream is seekable, by repositioning the stream back to
where it was, possibly by just calling sendfile with an offset
arg. This lets us return -EAFNOSUPPORT instead of -EIO in some
cases, which lets our callers try again with user-mode code.
* modules/crypto/af_alg (Depends-on): Depend on fseeko and ftello
instead of on fflush and lseek.
---
ChangeLog | 9 +++++++++
lib/af_alg.c | 43 ++++++++++++++++++++-----------------------
lib/af_alg.h | 15 +++++++++------
modules/crypto/af_alg | 4 ++--
4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3a9dc63ec..4eb095858 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2018-05-09 Paul Eggert <***@cs.ucla.edu>

+ af_alg: recover better from crypto failures
+ * lib/af_alg.c (afalg_stream): Recover from crypto failures if the
+ input stream is seekable, by repositioning the stream back to
+ where it was, possibly by just calling sendfile with an offset
+ arg. This lets us return -EAFNOSUPPORT instead of -EIO in some
+ cases, which lets our callers try again with user-mode code.
+ * modules/crypto/af_alg (Depends-on): Depend on fseeko and ftello
+ instead of on fflush and lseek.
+
af_alg: distiguish I/O errors better
* lib/af_alg.c (afalg_buffer, afalg_stream): Return -EAFNOSUPPORT,
not -EIO, if it’s OK for the caller to try again with user-mode code.
diff --git a/lib/af_alg.c b/lib/af_alg.c
index cfc31529a..677a8e340 100644
--- a/lib/af_alg.c
+++ b/lib/af_alg.c
@@ -103,33 +103,27 @@ afalg_stream (FILE *stream, const char *alg,
if (ofd < 0)
return ofd;

- /* if file is a regular file, attempt sendfile to pipe the data. */
+ /* If STREAM's size is known and nonzero and not too large, attempt
+ sendfile to pipe the data. The nonzero restriction avoids issues
+ with /proc files that pretend to be empty, and lets the classic
+ read-write loop work around an empty-input bug noted below. */
int fd = fileno (stream);
int result = 0;
struct stat st;
- if (fstat (fd, &st) == 0
+ off_t nseek = 0, off = ftello (stream);
+ if (0 <= off && fstat (fd, &st) == 0
&& (S_ISREG (st.st_mode) || S_TYPEISSHM (&st) || S_TYPEISTMO (&st))
- && 0 < st.st_size && st.st_size <= SYS_BUFSIZE_MAX)
+ && off < st.st_size && st.st_size - off < SYS_BUFSIZE_MAX)
{
- /* Make sure the offset of fileno (stream) reflects how many bytes
- have been read from stream before this function got invoked.
- Note: fflush on an input stream after ungetc does not work as expected
- on some platforms. Therefore this situation is not supported here. */
- if (fflush (stream))
- result = -EIO;
- else
- {
- off_t nbytes = st.st_size - lseek (fd, 0, SEEK_CUR);
- /* On Linux < 4.9, the value for an empty stream is wrong (all 0).
- See <https://patchwork.kernel.org/patch/9308641/>. */
- if (nbytes <= 0 || sendfile (ofd, fd, NULL, nbytes) != nbytes)
- result = -EAFNOSUPPORT;
- }
+ off_t nbytes = st.st_size - off;
+ if (sendfile (ofd, fd, &off, nbytes) != nbytes)
+ result = -EAFNOSUPPORT;
}
else
{
- /* sendfile not possible, do a classic read-write loop. */
- int non_empty = 0;
+ /* sendfile not possible, do a classic read-write loop.
+ Defer to glibc as to whether EOF is sticky; see
+ <https://sourceware.org/bugzilla/show_bug.cgi?id=19476>. */
for (;;)
{
char buf[BLOCKSIZE];
@@ -138,25 +132,28 @@ afalg_stream (FILE *stream, const char *alg,
{
/* On Linux < 4.9, the value for an empty stream is wrong (all 0).
See <https://patchwork.kernel.org/patch/9308641/>. */
- if (!non_empty)
+ if (nseek == 0)
result = -EAFNOSUPPORT;

if (ferror (stream))
result = -EIO;
break;
}
- non_empty = 1;
+ nseek -= size;
if (send (ofd, buf, size, MSG_MORE) != size)
{
- result = -EIO;
+ result = -EAFNOSUPPORT;
break;
}
}
}

if (result == 0 && read (ofd, resblock, hashlen) != hashlen)
- result = -EIO;
+ result = -EAFNOSUPPORT;
close (ofd);
+ if (result == -EAFNOSUPPORT && nseek != 0
+ && fseeko (stream, nseek, SEEK_CUR) != 0)
+ result = -EIO;
return result;
}

diff --git a/lib/af_alg.h b/lib/af_alg.h
index 018fa221a..429ec210d 100644
--- a/lib/af_alg.h
+++ b/lib/af_alg.h
@@ -1,4 +1,4 @@
-/* af_alg.h - Compute message digests from file streams.
+/* af_alg.h - Compute message digests from file streams and buffers.
Copyright (C) 2018 Free Software Foundation, Inc.

This program is free software; you can redistribute it and/or modify it
@@ -61,11 +61,12 @@ int
afalg_buffer (const char *buffer, size_t len, const char *alg,
void *resblock, ssize_t hashlen);

-/* Compute a message digest of the contents of a file.
+/* Compute a message digest of data read from STREAM.

- STREAM is an open file stream. Regular files are handled more efficiently.
- The contents of STREAM from its current position to its end will be read.
- The case that the last operation on STREAM was an 'ungetc' is not supported.
+ STREAM is an open file stream. The last operation on STREAM should
+ not be 'ungetc', and if STREAM is also open for writing it should
+ have been fflushed since its last write. Read from the current
+ position to the end of STREAM. Handle regular files efficently.

ALG is the message digest algorithm; see the file /proc/crypto.

@@ -82,7 +83,9 @@ afalg_buffer (const char *buffer, size_t len, const char *alg,
sha512 | 64

If successful, fill RESBLOCK and return 0.
- Upon failure, return a negated error number. */
+ Upon failure, return a negated error number.
+ Unless returning 0 or -EIO, restore STREAM's file position so that
+ the caller can fall back on some other method. */
int
afalg_stream (FILE *stream, const char *alg,
void *resblock, ssize_t hashlen);
diff --git a/modules/crypto/af_alg b/modules/crypto/af_alg
index 0c498e05c..519bab7bf 100644
--- a/modules/crypto/af_alg
+++ b/modules/crypto/af_alg
@@ -8,8 +8,8 @@ lib/sys-limits.h
m4/af_alg.m4

Depends-on:
-fflush [test $USE_AF_ALG = 1]
-lseek [test $USE_AF_ALG = 1]
+fseeko [test $USE_AF_ALG = 1]
+ftello [test $USE_AF_ALG = 1]
sys_socket
sys_stat
--
2.17.0
Loading...