lkml.org 
[lkml]   [1998]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectProposed kernel changes to dquot 5.6.0
Date
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]
\
 
 \ /
  Last update: 2005-03-22 13:41    [W:0.025 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site