Quota for root/setting grace period (was: Error (typo) in quotacheck(8))

[Edgar Fuß <ef%math.uni-bonn.de@localhost>]

I think there are some more errors in the quota code.

> zero is root, root gets no quotas by definition, so calculating grace
> times would be fairly meaningless.
It seems to be somewhat more complicated than that, and I think the're two
errors in that area (apart from not consulting kauth).

Quotas are not enforced for a process running as root, nevertheless
1) both quotas limits and usage are recorded for root,
2) the time fields of quota entry 0 are abused for holding the grace period,
3) quotas are enforced for files owned by root.

(In case someone wants to implement kauth usage by the quota system, the
test for not enforcing quotas for root processes is in sys/ufs/ufsufs_quota.c,
line 136:
        if ((flags & FORCE) == 0 &&
            (cred != NOCRED && cred->cr_uid != 0)) {
)

There are two problems with keeping the grace periods in dquot 0 (they are
copied into the ufsmount structure in quotaon()):

Fist issue: In case uid 0's quotas are exceeded by a non-root process
(I checked with a mode 666 file owned by root), the dq_[bi]time fields of
dquot entry 0 are set to the end of the grace period, thus overwriting the
grace interval.
So from then on, people are allowed insanely high grace intervals.
It looks like chkdqchg() and chkiqchg() should refrain from touching id 0's
time fields:

                if (dq->dq_curblocks < dq->dq_bsoftlimit) {
+                       if (dq->dq_id != 0) /* don't mangle grace period */
                        dq->dq_btime = time.tv_sec + ip->i_ump->um_btime[type];

resp.

                if (dq->dq_curinodes < dq->dq_isoftlimit) {
+                       if (dq->dq_id != 0) /* don't mangle grace period */
                        dq->dq_itime = time.tv_sec + ip->i_ump->um_itime[type];

Is this correct?


Second issue: If you change the grace interval with edquota -t, that calls
setquota() with id=0 (that usage being the reason for the dq->dq_id != 0 check
I asked about), but setquota() will only write the value into dquot entry 0,
not the mount structure, so changing the grace interval will only take effect
if you switch quotas off and back on again.

I guess this can be fixed by changing this part of setquota()

        if (dq->dq_id != 0) {

to

        if (dq->dq_id == 0) {
                /* copy the new grace periods to the mount structure */
                ump->um_btime[type] = dq->dq_btime != 0 ? dq->dq_btime : 
MAX_DQ_TIME;
                ump->um_itime[type] = dq->dq_itime != 0 ? dq->dq_itime : 
MAX_IQ_TIME;
        } else {

but I'm not sure that is correct. The check for dq_?time != 0 and using
MAX_?QTIME instead of 0 is supposed to resemble quotaon()s behaviour.

Comments?