cvs commit: src/sys/ufs/ufs ufs_quota.c

[Bruce Evans]

On Thu, 1 Feb 2007, Mike Pritchard wrote:

> mpp         2007-02-01 01:01:57 UTC
>
>  FreeBSD src repository
>
>  Modified files:
>    sys/ufs/ufs          ufs_quota.c
>  Log:
>  Disallow negative UIDs when processing quotactl options.

Er, uids are unsigned, so they cannot be negative.

The function actually takes a u_long id and now uses a bogus cast
((int)id) to check for "negative" values.  The correct check is
something like (id <= QUOTA_ID_MAX).  ((int)id) would work to restrict
the id to <= INT_MAX due to previous bogusness (*), but I don't see
the point of that.  If ints are 32-bits then id = INT_MAX gives an
offset that is about half as huge as id = UINT_MAX (64G?), and if
ints are 64 bits then id = INT_MAX and id = UINT_MAX both give
physically impossible offsets.  Is the problem with negative ids
mainly that they are standard for nfs without maproot?

(*) The id started as "int id" in the user API, but got punned in
several stages to "uid_t uid" in the kernel quotactl() and
vfs_quotactl_t.  ufs_quotactl() now starts by punning it half way back
(uid_t id).  As a result of this, any overflow for converting uids and
gids to ids has already occurred at the syscall boundary, and most
overflows in the kernel consist of overflowing back to the original
value (uid_t -> int -> uid_t kernel) and back to the already-overflowed
value (uid_t -> int -> uid_t -> int).  Since uid_t is uint32_t and int
is 32 bits 2's complement on all supported systems, the only overflows
of ids that occur now are from huge positive to negative and back.
However, if the id is initially -1, then all overflows are in the
kernel (-1 -> u/gid -> ...).  Also, an u/gid 0xffffffff overflows to
-1 at the syscall boundary and then gets corrrupted to the real u/gid
in the kernel.

The user API is only correct if ints have more bits that u/gids, so
that conversion of u/gids to ints doesn't overflow and -1 isn't in-band.
It was correct before 4.4BSD on systems with >= 17-bit ints, since
u/gid_t was only 16 bits until then.

The patch has some style bugs (misformatted comment, unlike the other
XXX comments about "negative" uids).

Bruce