[UNIKRAFT PATCH 2/3] lib/vfscore: Change preadv and pwritev fd error handling.

[Alexander Jung <a.jung@xxxxxxxxxxx>]

This commit discards the `has_error` method in favour of in-line checks against 
the file descriptor.  In BSD's internal implementation, the error is simply 
passed upwards from `dofilewrite` and `dofileread.  Additionally, the 
`has_error` method would return a false positive in the circumstance where an
error was non-zero and did not constitute a EWOULDBLOCK or EINTR and the number 
of bytes read was greater than zero.  This scenario was discovered when a 
buffer greater than the number of bytes available was read from a vnop read op 
which returns the number of bytes read (as per POSIX requirements).

Additional checks inline with FreeBSD's[0][1] implementation are for whether 
the fd is seekable; the offset of the file is non-zero; and, the file type is 
not a character device.

[0]: 
https://github.com/freebsd/freebsd/blob/6af4a8cb884975f65a51b3b6d766e084faf55d4f/sys/kern/sys_generic.c#L320
[1]: 
https://github.com/freebsd/freebsd/blob/6af4a8cb884975f65a51b3b6d766e084faf55d4f/sys/kern/sys_generic.c#L522

Signed-off-by: Alexander Jung <alexander.jung@xxxxxxxxx>
---
 lib/vfscore/main.c | 61 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c
index 3dcbd51..0cbcd21 100644
--- a/lib/vfscore/main.c
+++ b/lib/vfscore/main.c
@@ -294,19 +294,6 @@ UK_TRACEPOINT(trace_vfs_pread, "%d %p 0x%x 0x%x", int, 
void*, size_t, off_t);
 UK_TRACEPOINT(trace_vfs_pread_ret, "0x%x", ssize_t);
 UK_TRACEPOINT(trace_vfs_pread_err, "%d", int);
 
-// In BSD's internal implementation of read() and write() code, for example
-// sosend_generic(), a partial read or write returns both an EWOULDBLOCK error
-// *and* a non-zero number of written bytes. In that case, we need to zero the
-// error, so the system call appear a successful partial read/write.
-// In FreeBSD, dofilewrite() and dofileread() (sys_generic.c) do this too.
-static inline int has_error(int error, int bytes)
-{
-       /* TODO: OSv checks also for ERESTART */
-       return error && (
-                       (bytes == 0) ||
-                       (error != EWOULDBLOCK && error != EINTR));
-}
-
 ssize_t pread(int fd, void *buf, size_t count, off_t offset)
 {
        trace_vfs_pread(fd, buf, count, offset);
@@ -378,14 +365,30 @@ ssize_t preadv(int fd, const struct iovec *iov, int 
iovcnt, off_t offset)
        if (error)
                goto out_errno;
 
+       /* Check if the file is indeed seekable. */
+       if (!(fp->f_vfs_flags & UK_VFSCORE_NOPOS)) {
+               error = ESPIPE;
+               goto out_errno_fdrop;
+       }
+       /* Check if the file has not already been read and that is not a
+        * character device. */
+       else if (fp->f_offset < 0 && \
+               (fp->f_dentry == NULL || fp->f_dentry->d_vnode->v_type != 
VCHR)) {
+               error = EINVAL;
+               goto out_errno_fdrop;
+       }
+       /* Otherwise, try to read the file. */
        error = sys_read(fp, iov, iovcnt, offset, &bytes);
-       fdrop(fp);
+       if (error < 0)
+               goto out_errno_fdrop;
 
-       if (has_error(error, bytes))
-               goto out_errno;
+       fdrop(fp);
        return bytes;
 
-       out_errno:
+out_errno_fdrop:
+       fdrop(fp);
+
+out_errno:
        errno = error;
        return -1;
 }
@@ -414,15 +417,31 @@ ssize_t pwritev(int fd, const struct iovec *iov, int 
iovcnt, off_t offset)
        if (error)
                goto out_errno;
 
+       /* Check if the file is indeed seekable. */
+       if (!(fp->f_vfs_flags & UK_VFSCORE_NOPOS)) {
+               error = ESPIPE;
+               goto out_errno_fdrop;
+       }
+       /* Check if the file has not already been written to and that it is not 
a
+        * character device. */
+       else if (fp->f_offset < 0 && \
+               (fp->f_dentry == NULL || fp->f_dentry->d_vnode->v_type != 
VCHR)) {
+               error = EINVAL;
+               goto out_errno_fdrop;
+       }
+       /* Otherwise, try to read the file. */
        error = sys_write(fp, iov, iovcnt, offset, &bytes);
-       fdrop(fp);
+       if (error < 0)
+               goto out_errno_fdrop;
 
-       if (has_error(error, bytes))
-               goto out_errno;
+       fdrop(fp);
        trace_vfs_pwritev_ret(bytes);
        return bytes;
 
-       out_errno:
+out_errno_fdrop:
+       fdrop(fp);
+
+out_errno:
        trace_vfs_pwritev_err(error);
        errno = error;
        return -1;
-- 
2.11.0