possible bug in quota format v2 support

PATCH In review Niu YaWei 3 participants 3 messages Mar 31, 2005 – Apr 1, 2005 VFS

lore

Share & tools

lore

Niu YaWei

author

2005-03-31 13:24 UTC #0 lore raw

PATCH 1/1 possible bug in quota format v2 support

Hi Jan,

I'm afraid that there is a bug in quota format v2 delete qentry. (The root block shouldn't be put into free blk list even if there isn't any entry in quota file, right?)

This one line patch may fix it.

Thanks.

- Niu

fs/quota_v2.c 2005-03-31 21:08:30.012641840 +0800 renamed from fs/quota_v2.c 2005-01-21 16:47:34.000000000 +0800 +0 -0

@@ -529,7 +529,8 @@ static int remove_tree(struct dquot *dqu
               int i;
               ref[GETIDINDEX(dquot->dq_id, depth)] = cpu_to_le32(0);
               for (i = 0; i < V2_DQBLKSIZE && !buf[i]; i++);  /* Block

got empty? */

-               if (i == V2_DQBLKSIZE) {
+               /* don't put the root block into the free block list */
+               if (i == V2_DQBLKSIZE && *blk != V2_DQTREEOFF) {
                       put_free_dqblk(filp, dquot->dq_type, buf, *blk);
                       *blk = 0;
               }

Jan Kara

reviewer

2005-03-31 14:22 UTC #1 lore raw

Hello,

Quoted 5 lines

I'm afraid that there is a bug in quota format v2 delete qentry. (The root block shouldn't be put into free blk list even if there isn't any entry in quota file, right?)
This one line patch may fix it.

Such case should not actually occur in normal operation but I agree
that with your patch the code will handle errors more gracefuly. Your patch seems to have wrapped lines and substituted tabs for spaces (please try to persuade your mail agent not to wrap lines and substitute tabs next time or just send the patch as an attachment) so I rediffed it
- the result is attached. Andrew please apply the attached patch.

Thanks for spotting it
Honza

Signature · 2 lines

Jan Kara <[email protected]>
SuSE CR Labs

Don't put root block of quota tree to the free list (when quota file is completely empty). That should not actually happen anyway (somebody should get accounted for the filesystem root and so quota file should never be empty) but better prevent it here than solve magical quota file corruption...

From: Niu YaWei <[email protected]>

Signed-off-by Jan Kara <[email protected]>

fs/quota_v2.c +2 -1

diff -rupX /home/jack/.kerndiffexclude linux-2.6.11/fs/quota_v2.c linux-2.6.11-v2fix/fs/quota_v2.c
@@ -503,7 +503,8 @@ static int remove_tree(struct dquot *dqu
 		int i;
 		ref[GETIDINDEX(dquot->dq_id, depth)] = cpu_to_le32(0);
 		for (i = 0; i < V2_DQBLKSIZE && !buf[i]; i++);	/* Block got empty? */
-		if (i == V2_DQBLKSIZE) {
+		/* Don't put the root block into the free block list */
+		if (i == V2_DQBLKSIZE && *blk != V2_DQTREEOFF) {
 			put_free_dqblk(sb, type, buf, *blk);
 			*blk = 0;
 		}

Smets Jan

2005-04-01 12:31 UTC #2 lore raw

Quoted 33 lines · Jan Kara

Hello,
Quoted 5 lines
I'm afraid that there is a bug in quota format v2 delete qentry. (The root block shouldn't be put into free blk list even if there isn't any entry in quota file, right?)
This one line patch may fix it.
Such case should not actually occur in normal operation but I agree
that with your patch the code will handle errors more gracefuly. Your patch seems to have wrapped lines and substituted tabs for spaces (please try to persuade your mail agent not to wrap lines and substitute tabs next time or just send the patch as an attachment) so I rediffed it
- the result is attached. Andrew please apply the attached patch.
Thanks for spotting it
Honza
Signature · 12 lines
Jan Kara <[email protected]>
SuSE CR Labs

Don't put root block of quota tree to the free list (when quota file is completely
empty). That should not actually happen anyway (somebody should get accounted for
the filesystem root and so quota file should never be empty) but better prevent it
here than solve magical quota file corruption...

From: Niu YaWei <[email protected]>
Signed-off-by: Jan Kara <[email protected]>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.11/fs/quota_v2.c linux-2.6.11-v2fix/fs/quota_v2.c

fs/quota_v2.c static int remove_tree(struct dquot *dqu · line 510

Quoted 9 lines

fs/quota_v2.c +2 -1

@@ -503,7 +503,8 @@ static int remove_tree(struct dquot *dqu
 		int i;
 		ref[GETIDINDEX(dquot->dq_id, depth)] = cpu_to_le32(0);
 		for (i = 0; i < V2_DQBLKSIZE && !buf[i]; i++);	/* Block got empty? */
-		if (i == V2_DQBLKSIZE) {
+		/* Don't put the root block into the free block list */
+		if (i == V2_DQBLKSIZE && *blk != V2_DQTREEOFF) {
 			put_free_dqblk(sb, type, buf, *blk);
 			*blk = 0;
 		}

Well done Jan :-)

I've posted an update of my problem with the quotaoff oops:

http://bugme.osdl.org/show_bug.cgi?id=2698

The BUG is at fs/dquot.c:425

Thanks again.

Signature · 2 lines

Smets Jan
[email protected]