Messages in this thread | | | From | Chris Rankin <> | Subject | Proposed kernel changes to dquot 5.6.0 | Date | Fri, 9 Jan 1998 08:53:50 -0500 |
| |
Hi,
I have been experimenting with Linux disk quotas v5.6.0, and have discovered several flaws. I am listing them below in order of decreasing confidence so that someone else can verify my hacking :-). All changes are w.r.t. linux/fs/dquot.c
a) quotactl() can only retrieve quota data from ONE of the groups that you belong to: This problem is in sys_quotactl(). The kernel verifies the requested group-id against current->gid instead of using in_group_p(). The code should read:
... case Q_GETQUOTA: if (((type == USRQUOTA && current->uid != id) || (type == GRPQUOTA && !in_group_p(id))) && !fsuser()) return(-EPERM); break; ...
b) quotactl() never returns a useful error code when it fails to turn disk quotas off: The error code for quotactl(QCMD(Q_QUOTAOFF,... comes directly from the quota_off() routine, which always returns 0. The only calls to quota_off() that I have found in the kernel completely ignore this return code (not surprisingly :-) and so I think that it is safe to be a little more friendly to the person on the other end of quotactl(). To this end, I suggest altering quota_off() as follows:
... /* ** Fail with an error code if the device is bad ... */ if ( !(vfsmnt = lookup_vfsmnt(dev)) ) return -ENODEV;
for (cnt = 0; cnt < MAXQUOTAS; cnt++) { if (type != -1 && cnt != type) continue;
/* ** Return an error if the device doesn't have a quota enabled - unless we are turning ALL quotas off (type==-1) */ if ( !(vfsmnt->mnt_quotas[cnt]) ) { if (type == -1) continue; return -ESRCH; } ...
c) quotactl(QMCD(Q_SYNC,..),NULL,...). does nothing. When sys_quotactl() sees a NULL device for Q_SYNC, it is supposed to sync ALL devices. To this end it passes 0 (NODEV) to the sync_dquots() procedure. sync_dquots() then loops over its list of dquot structures (TWICE ???!! I don't understand why the loop is for nr_dquots*2 !!!???) looking for quotas to write to disk However, since the *empty* quota structures in the list have dquot->dq_dev == NODEV, the current logic skips the dev==NODEV case entirely. I propose the following:
... for (i = nr_dquots; i > 0; i--, dquot = dquot->dq_next) { if (dquot->dq_count == 0 || ((dev == NODEV) != (dquot->dq_dev != dev))) continue; ...
This logic accepts EITHER quotas on the specific device you requested (X)OR quotas on any device which isn't NODEV. It is worth pointing out here that sys_sync() in linux/fs/buffer.c indirectly passes sync_dquots() a device-number of 0, and then expects it to write everything to disk (oops!).
d) Eliminated redundant code in check_bdq() and check_idq().
I have successfully compiled these changes into my 2.0.33 kernel and it all seems to work. I would, however, appreciate a second/third/fourth opinion.
Cheers, Chris. [unhandled content-type:application/octet-stream] | |