Page MenuHomeFreeBSD

namei: preserve errors from fget_cap_locked
ClosedPublic

Authored by kevans on Feb 3 2020, 3:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 1:52 PM
Unknown Object (File)
Thu, Nov 28, 6:05 AM
Unknown Object (File)
Tue, Nov 26, 1:00 PM
Unknown Object (File)
Nov 22 2024, 10:36 PM
Unknown Object (File)
Oct 31 2024, 5:08 AM
Unknown Object (File)
Oct 14 2024, 8:43 PM
Unknown Object (File)
Sep 20 2024, 12:58 AM
Unknown Object (File)
Sep 20 2024, 12:57 AM
Subscribers

Details

Summary

Most notably, we want to make sure we don't clobber any capabilities-related errors. This is a regression from r357412 (O_SEARCH) that was picked up from the capsicum tests.

PR: 243839

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/kern/vfs_lookup.c
463–466 ↗(On Diff #67703)

To me a nested

if (error == 0) {
        if (dfp->f_vnode == NULL)
                error = ENOTDIR;
        else
                error = EBADF;
}

is easier to follow

kib added inline comments.
sys/kern/vfs_lookup.c
463–466 ↗(On Diff #67703)

I would do it diferently:

   if (error != 0) {
      /* Preserve error */
   } else if (...f_ops == &badfileops} {
     error = EBADF;
   } else if (...f_vnode == NULL) {
     error = EINVAL;
  } else {
      dp = dfp->f_vnode;
...

Note tht I restored errors from fgetvp_rights.

This revision is now accepted and ready to land.Feb 3 2020, 6:20 PM
This revision was automatically updated to reflect the committed changes.

I would do it differently

Ah yes, this is better.

(The recent OpenSMTPD vulnerability provides a good example of the need to consider this carefully and get it right from both correctness & readability.)