Date: Wed, 28 Jan 2015 20:55:57 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Robert Watson <rwatson@freebsd.org> Cc: svn-src-head@freebsd.org, Baptiste Daroussin <bapt@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r277652 - in head/usr.sbin/pw: . tests Message-ID: <20150128195647.E1974@besplex.bde.org> In-Reply-To: <alpine.BSF.2.11.1501272326010.53416@fledge.watson.org> References: <201501241913.t0OJD4xT039188@svn.freebsd.org> <20150125155254.V1007@besplex.bde.org> <alpine.BSF.2.11.1501272326010.53416@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 27 Jan 2015, Robert Watson wrote:
> On Sun, 25 Jan 2015, Bruce Evans wrote:
>
>> Negative ids have historical abuses in places like mountd. mountd still
>> hard-codes -2 and -2 for the default uid and gid of an unprivileged user.
>> ...
>
> I'm sure it goes without saying, but for those that don't know (i.e., some
> subset of people who are not Bruce):
>
> (-1) has a defined value both for our system-call interface (chown(2),
> fchown(2), etc, use (-1) to indicate that no change is requested).
>
> This is also used inside the kernel to similar end, where VNOVAL also takes
> on a value of (-1).
>
> This problem also used to exist in NFS, where in NFSv2, (-1) was also used to
> indicate which fields not to update, but this was fixed in NFSv3 by
> introducing discriminated unions.
>
> I personally find myself a fan of fixing (eliminating) VNOVAL, but in the end
> it would likely just be disruptive and confusing.
VNOVAL is kernel-only, so it need not limit values. But then you would
have to escape the value that it usurps.
ffs uses VNOVAL for (quoting an old version):
% if (vap->va_rdev != VNOVAL) {
% if ((vap->va_type != VNON) || (vap->va_nlink != VNOVAL) ||
% (vap->va_fsid != VNOVAL) || (vap->va_fileid != VNOVAL) ||
% (vap->va_blocksize != VNOVAL) || (vap->va_rdev != VNOVAL) ||
% ((int)vap->va_bytes != VNOVAL) || (vap->va_gen != VNOVAL)) {
Some of these are magic or kernel-only or otherwise have plenty of out
of band values. va_nlink is most interesting. It is only 16 bits signed.
If it were not signed, then it couldn't represent VNOVAL. {LINK_MAX} is
hard-coded as LINK_MAX = 32767. The hard-coded type and value are both
broken for file systems that have larger or smaller limits. Removing
the definition of LINK_MAX would fix this for limits up to 32767. The
limit in ext2fs is 32000 and the limit in ext2fs is 65000. 65000 is
not supported in FreeBSD. It doesn't fit in nlink_t. It leaves 536
out of band vales in a 16-bit unsigned nlink_t. One of these can be
VNOVAL after type puns to make VNOVAL 65535.
% if (vap->va_flags != VNOVAL) {
Flags have enough spare bits. va_flags is u_long, so type puns occur
implicitly in this comparison. The flags type(s) have even more design
botches than older types. A mixture of signed int, uint32_t and u_long
is used for flags variables.
% * Go through the fields and update iff not VNOVAL.
% if (vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL) {
Here the type puns are done explicitly (so they are not really puns).
The types here are unsigned, so there are no unportabilities in these
conversions. It is just accidental that VNOVAL = -1 matches the POSIX
magic number -1 (even before the conversions). The code depends on this.
% if (vap->va_size != VNOVAL) {
A size of -1 is impossible, but va_size is unsigned so the usual type
puns occur and the out of band value is actually 0xffffffffffffffff.
That should be large enough for anyone. It is currently physically
impossible, and also theoretically impossible, since file offsets are
signed so they can only reach half as high.
% if (vap->va_atime.tv_sec != VNOVAL ||
% vap->va_mtime.tv_sec != VNOVAL ||
% vap->va_birthtime.tv_sec != VNOVAL) {
% if (vap->va_atime.tv_sec != VNOVAL)
% if (vap->va_mtime.tv_sec != VNOVAL)
% if (vap->va_birthtime.tv_sec != VNOVAL &&
% if (vap->va_atime.tv_sec != VNOVAL) {
% if (vap->va_mtime.tv_sec != VNOVAL) {
% if (vap->va_birthtime.tv_sec != VNOVAL &&
time_t is signed, so the out of band value is 1 second before the Epoch,
not (via type puns) 1 second before the time_t's roll over. This magic
value is also reserved for reporting errors in C (for mktime(), etc.).
Error handling is especially difficult for APIs like mktime(), strtol(),
and some syscalls since the error value is also a supported in-band.
For strtol(), it is a very normal value. In C, the representation can
be anything so the error value can be for any time. Implementations
should avoid using it for a useful time, but portable applications
cannot depend on this. In POSIX, the error value must be 1 second
before the Epoch and that is an interesting time.
% if (vap->va_mode != (mode_t)VNOVAL) {
The 16-bit mode_t is not quite full (type 0170000 is free), so VNOVAL
is out of band. Oops. There would be no problem even if mode_t were full,
since it gets promoted and since it is unsigned it cannot promote to -1.
Similarly for nlink_t if it were unsigned.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150128195647.E1974>