Btrfs devel - Nov 2012 - [RFC PATCH] Btrfs-progs: disable qgroupid 0 for quota_tree

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

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.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 cmds-qgroup.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 70019d0..dfff1b9 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -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 









--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 08/11/12 20:55, Miao Xie wrote:
> 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
-- 
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On fri, 09 Nov 2012 10:33:52 +1100, Chris Samuel wrote:
> On 08/11/12 20:55, Miao Xie wrote:
> 
>> 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.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Miao,

On Mon, November 12, 2012 at 03:21 (+0100), Miao Xie
wrote:
> On fri, 09 Nov 2012 10:33:52 +1100, Chris Samuel wrote:
>> On 08/11/12 20:55, Miao Xie wrote:
>>
>>> 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.
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, 12 Nov 2012 07:25:08 +0100, Jan Schmidt wrote:
>>>> 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.
>>
>> 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.
You''ve got a point there. I''ll send out the kernel patch.

Thanks
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html