Page MenuHomeFreeBSD

Add missing permissions in open(2) calls with O_CREAT.
ClosedPublic

Authored by brooks on Jul 16 2019, 4:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 26 2024, 11:55 AM
Unknown Object (File)
Jan 27 2024, 2:44 PM
Unknown Object (File)
Jan 6 2024, 5:35 PM
Unknown Object (File)
Dec 22 2023, 11:22 PM
Unknown Object (File)
Dec 16 2023, 3:36 PM
Unknown Object (File)
Dec 11 2023, 5:33 AM
Unknown Object (File)
Nov 22 2023, 9:05 AM
Unknown Object (File)
Nov 15 2023, 10:15 AM
Subscribers

Details

Summary

The test suite contains a large number of instances of things like:

fd = open(path, O_RDWR | O_CREAT);

When O_CREAT is specified, the third, variadic argument is
required as the permission. If on is not passed, then depending
on the ABI, either the contents of the third argument register
or some arbitrary stuff on the stack will be used as the permission.
(On some out-of-tree ABIs, accessing nonexistant variadic arguments
leads to a fault and a failed test.)

Diff Detail

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

Event Timeline

LGTM, though ngie sometimes objects to changes to these files because of the upstream divergence. It's a shame that neither the compiler nor Coverity caught this error. However, it's worth noting that Coverity _does_ catch 43 other errors in contrib/netbsd-tests; it might be worth proactively fixing those. BTW, what "out-of-tree ABIs" generate a fault?

This revision is now accepted and ready to land.Jul 16 2019, 5:23 PM

NetBSD just committed these changes and some others just now. I'm waiting for cvsweb to update so I can check what was actually committed.

In CheriABI (hardware fat pointers), va_list is a bounded array on the stack. In this case it ends up as a zero-length array so reading mode from it yields a fault rather then treating stack or register garbage as a mode.

This is totally fine —- as long as the upstream commits are referenced, I’m ok with that (my concern is with changes not being communicated beforehand and having to reverse engineer where and how to upstream them to the other projects, since things tend to change from time to time).

It’s about time for me to pull down a new snapshot soon and reconcile the changes that have been made in the past couple years.

This revision was automatically updated to reflect the committed changes.