Page MenuHomeFreeBSD

Fix lseek() so that it never returns errnos not listed on the man page, such as ENOTTY

Authored by rmacklem on Aug 17 2019, 3:24 AM.



VOP_IOCTL() can return errors such as ENOTTY, which are not listed as errnos for lseek(2)
on either the man page nor in POSIX.

The trivial patch maps all errnos not listed in the man page (and POSIX) to EINVAL, which
is listed. This is done mainly for the ENOTTY case, but serves as a "catch-all" for any other
non-standard errnos that might be returned by the VOP_IOCTL().

Test Plan

A test program was run on an NFS mount (which does not have a VOP_IOCTL() and, as such, the
VOP_IOCTL() call returns ENOTTY) which does lseek(SEEK_DATA/SEEK_HOLE), to see that it gets
errno == EINVAL instead of ENOTTY.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rmacklem created this revision.Aug 17 2019, 3:24 AM
markj added a comment.Aug 20 2019, 5:08 PM

I understand that the intent is to filter out unexpected error values, but I think it would be better to simply restrict the check to ENOTTY: if our code starts returning an unexpected error number (i.e., not one of the documented error numbers and not ENOTTY), and we map it to EINVAL, userspace will get a documented error number for an undocumented reason, which IMHO isn't much better. Preserving the original error number will make it easier to determine where the error is coming from, which will inform whether it becomes a documented error for lseek().

Actually, I only mapped ENOTTY when I first did the patch. The only reason I changed it
is that any error returned by VOP_GETATTR() also gets returned to lseek(2). For NFS,
this could be EACCES, for example, since file permissions are checked for every operation
on NFS, nit just at open(2) time.

However, I have no problem going to just mapping ENOTTY.

I update the patch tomorrow.
I'll also take a closer look at what errors VOP_GETATTR() can return.

rmacklem updated this revision to Diff 61097.Aug 21 2019, 10:28 PM

Updated the patch to only map ENOTTY to EINVAL, per Mark's comments.

markj accepted this revision.Aug 21 2019, 10:33 PM
This revision is now accepted and ready to land.Aug 21 2019, 10:33 PM
This revision was automatically updated to reflect the committed changes.