Skip to contentSkip to conversation

Btrfs-progs: disable qgroupid 0 for quota_tree

RFC PATCH In review Miao Xie 3 participants 5 messages Nov 8, 2012 – Nov 12, 2012 1 message not in the archive Btrfs file system

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Miao Xie

author
#1 lore raw

RFC PATCH 1/1 Btrfs-progs: disable qgroupid 0 for quota_tree

From: Wang Shilong <[email protected]>

In kernel, qgroupid 0 is a special number when we run the quota group limit command.

So, we should not be able to create a quota group whose id is 0, otherwise the kernel can't deal with it. Fix it.

---
 cmds-qgroup.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
cmds-qgroup.c +4 -0
index 70019d0..dfff1b9 100644
@@ -86,6 +86,10 @@ static int qgroup_create(int create, int argc, char **argv)
 	args.create = create;
 	args.qgroupid = parse_qgroupid(argv[1]);
 
+	if (!args.qgroupid) {
+		fprintf(stderr, "ERROR: qgroup 0 is not supported\n");
+		return 30;
+	}
 	fd = open_file_or_dir(path);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);

-- 1.7.7.6

#2 lore raw
Quoted 5 lines · Miao Xie

In kernel, qgroupid 0 is a special number when we run the quota group limit command.

So, we should not be able to create a quota group whose id is 0, otherwise the kernel can't deal with it. Fix it.

This is probably a stupid question - but if its not meant to be possible to create such a thing shouldn't this be fixed in the kernel (as well as here) to reject attempts from user space to create it?

Otherwise it's possible for a non-aware program (or a user who is playing) to still create it.

cheers,
Chris

Signature · 1 line
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC
author
#3 lore raw
Quoted 8 lines · Chris Samuel
Quoted 5 lines · Miao Xie

In kernel, qgroupid 0 is a special number when we run the quota group limit command.

So, we should not be able to create a quota group whose id is 0, otherwise the kernel can't deal with it. Fix it.

Quoted 6 lines

This is probably a stupid question - but if its not meant to be possible to create such a thing shouldn't this be fixed in the kernel (as well as here) to reject attempts from user space to create it?

Otherwise it's possible for a non-aware program (or a user who is playing) to still create it.

Right. It also should be fixed in the kernel side, the patch is coming.

But since we know which number is valid or not, it is better that we also check the arguments in the user tool before they are passed into the kernel. So, we can avoid trapping into the kernel, which will waste time, and output the error information as soon as possible.

Thanks
Miao

#4 lore raw

Hi Miao,

Quoted 16 lines · Miao Xie
Quoted 8 lines · Chris Samuel
Quoted 5 lines · Miao Xie

In kernel, qgroupid 0 is a special number when we run the quota group limit command.

So, we should not be able to create a quota group whose id is 0, otherwise the kernel can't deal with it. Fix it.

Quoted 6 lines

This is probably a stupid question - but if its not meant to be possible to create such a thing shouldn't this be fixed in the kernel (as well as here) to reject attempts from user space to create it?

Otherwise it's possible for a non-aware program (or a user who is playing) to still create it.

Quoted 6 lines

Right. It also should be fixed in the kernel side, the patch is coming.

But since we know which number is valid or not, it is better that we also check the arguments in the user tool before they are passed into the kernel. So, we can avoid trapping into the kernel, which will waste time, and output the error information as soon as possible.

Have the kernel return with errno EINVAL is the way everyone else (I know about) does this.

I would avoid doing a check more than once where ever possible. Primarily, because duplicating checks is error prone and in case of a kernel interface it complicates future changes.

-Jan

author
#5 lore raw
Quoted 20 lines · Jan Schmidt
Quoted 13 lines
Quoted 6 lines
Quoted 5 lines

In kernel, qgroupid 0 is a special number when we run the quota group limit command.

So, we should not be able to create a quota group whose id is 0, otherwise the kernel can't deal with it. Fix it.

Quoted 6 lines

This is probably a stupid question - but if its not meant to be possible to create such a thing shouldn't this be fixed in the kernel (as well as here) to reject attempts from user space to create it?

Otherwise it's possible for a non-aware program (or a user who is playing) to still create it.

Quoted 6 lines

Right. It also should be fixed in the kernel side, the patch is coming.

But since we know which number is valid or not, it is better that we also check the arguments in the user tool before they are passed into the kernel. So, we can avoid trapping into the kernel, which will waste time, and output the error information as soon as possible.

Quoted 6 lines

Have the kernel return with errno EINVAL is the way everyone else (I know about) does this.

I would avoid doing a check more than once where ever possible. Primarily, because duplicating checks is error prone and in case of a kernel interface it complicates future changes.

You've got a point there. I'll send out the kernel patch.

Thanks
Miao