afalg_stream is documented as "Read from the current position to the
end of STREAM." However, the current behaviour after sendfile() succeeds
is that the stream is in an unusable state (except for fclose): It still
has buffers allocated, but the underlying file descriptors is at a
different position that what would be consistent with the buffers.
You can see that by doing a getc() call afterwards.
Should we change af_alg.h, md5.h, sha1.h etc. to all say "The only valid
operation on STREAM afterwards is to close it."? I don't think this would
be good: it would be a restriction that could too easily trigger bugs in
the calling programs.
So it's better to make sure STREAM is in a consistent state afterwards.
This patch does it. In particular, it restores the fflush() call that Paul
had eliminated on 2018-05-10.
2018-06-24 Bruno Haible <br...@clisp.org>
af_alg: Fix state of stream after sendfile() succeeds.
* lib/af_alg.c (afalg_stream): Invoke fflush and lseek, to ensure that
the stream is correctly positioned afterwards.
* modules/crypto/af_alg (Depends-on): Add fflush.
* tests/test-digest.h (test_digest_on_files): Verify that after the
operation the stream is positioned at end of file.
diff --git a/lib/af_alg.c b/lib/af_alg.c
index 555eb2b..2753ab9 100644
--- a/lib/af_alg.c
+++ b/lib/af_alg.c
@@ -113,18 +113,47 @@ afalg_stream (FILE *stream, const char *alg,
int fd = fileno (stream);
int result;
struct stat st;
- off_t nseek = 0; /* Number of bytes to seek (backwards) in case of error. */
off_t off = ftello (stream);
if (0 <= off && fstat (fd, &st) == 0
&& (S_ISREG (st.st_mode) || S_TYPEISSHM (&st) || S_TYPEISTMO (&st))
&& off < st.st_size && st.st_size - off < SYS_BUFSIZE_MAX)
{
- off_t nbytes = st.st_size - off;
- result = sendfile (ofd, fd, &off, nbytes) == nbytes ? 0 : -EAFNOSUPPORT;
+ /* 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 - off;
+ if (sendfile (ofd, fd, &off, nbytes) == nbytes)
+ {
+ if (read (ofd, resblock, hashlen) == hashlen)
+ {
+ /* The input buffers of stream are no longer valid. */
+ if (lseek (fd, off, SEEK_SET) != (off_t)-1)
+ result = 0;
+ else
+ /* The file position of fd has not changed. */
+ result = -EAFNOSUPPORT;
+ }
+ else
+ /* The file position of fd has not changed. */
+ result = -EAFNOSUPPORT;
+ }
+ else
+ /* The file position of fd has not changed. */
+ result = -EAFNOSUPPORT;
+ }
}
else
{
/* sendfile not possible, do a classic read-write loop. */
+
+ /* Number of bytes to seek (backwards) in case of error. */
+ off_t nseek = 0;
+
for (;;)
{
char buf[BLOCKSIZE];
@@ -154,14 +183,14 @@ afalg_stream (FILE *stream, const char *alg,
break;
}
}
- }
- if (result == 0 && read (ofd, resblock, hashlen) != hashlen)
- {
- if (nseek == 0 || fseeko (stream, nseek, SEEK_CUR) == 0)
- result = -EAFNOSUPPORT;
- else
- result = -EIO;
+ if (result == 0 && read (ofd, resblock, hashlen) != hashlen)
+ {
+ if (nseek == 0 || fseeko (stream, nseek, SEEK_CUR) == 0)
+ result = -EAFNOSUPPORT;
+ else
+ result = -EIO;
+ }
}
close (ofd);
return result;
diff --git a/modules/crypto/af_alg b/modules/crypto/af_alg
index 2ad25fe..95273d4 100644
--- a/modules/crypto/af_alg
+++ b/modules/crypto/af_alg
@@ -9,6 +9,7 @@ m4/af_alg.m4
Depends-on:
c99 [test $USE_AF_ALG = 1]
+fflush [test $USE_AF_ALG = 1]
fseeko [test $USE_AF_ALG = 1]
ftello [test $USE_AF_ALG = 1]
sys_socket
diff --git a/tests/test-digest.h b/tests/test-digest.h
index 369ef9f..c58d3da 100644
--- a/tests/test-digest.h
+++ b/tests/test-digest.h
@@ -129,6 +129,12 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
fprintf (stderr, "\n");
exit (1);
}
+ /* Verify that fp is now positioned at end of file. */
+ if (getc (fp) != EOF)
+ {
+ fprintf (stderr, "%s left the stream not at EOF\n",
streamfunc_name);
+ exit (1);
+ }
fclose (fp);
free (digest - 1);
}
