[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <50DA6BC7.1000804@asianux.com>
Date: Wed, 26 Dec 2012 11:15:19 +0800
From: Chen Gang <gang.chen@...anux.com>
To: Theodore Ts'o <tytso@....edu>
CC: jack@...e.cz, akpm@...ux-foundation.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] fs/ext3: use kzalloc instead of kmalloc
于 2012年12月26日 02:48, Theodore Ts'o 写道:
> On Mon, Dec 24, 2012 at 01:28:53PM +0800, Chen Gang wrote:
>>
>> better to use kzalloc instead of kmalloc.
>> if acl_e->e_tag is neither ACL_USER, nor ACL_GROUP.
>> entry->e_id will not be initialized
>>
>> we can not say it is a bug, but suggest to initialize it, too.
>>
>> Signed-off-by: Chen Gang <gang.chen@...anux.com>
>
> This shouldn't be a problem, since if e_tag is not ACL_USER nor
> ACL_GROUP, the on-disk encoding does not include e_id at all.
>
ok, thanks. it is my fault.
:-)
> That being said, it looks to me there's another bug hiding here. The
> size of the extended attribute is calculated by ext3_acl_size(), and
> it looks totally wrong. For one thing, it caluclates the size of the
> xattr assuming all of the stored encoding ext3_acl_entry_short ---
> which would not be the case if we had a acl entry of type ACL_USER or
> ACL_GROUP.
>
> But if that were the case, it would mean that we would not be storing
> the full acl entry on disk, which would be a pretty horrible and
> obvious breakage.
>
checking the ext3_acl_size, it does not like what you said above.
but we can say, the design for ext3_acl_size is really not quit well.
(maybe can cause issue).
26 static inline size_t ext3_acl_size(int count)
27 {
28 if (count <= 4) {
29 return sizeof(ext3_acl_header) +
30 count * sizeof(ext3_acl_entry_short);
31 } else {
32 return sizeof(ext3_acl_header) +
33 4 * sizeof(ext3_acl_entry_short) +
34 (count - 4) * sizeof(ext3_acl_entry);
35 }
36 }
> I haven't had time to check this yet, but I wanted to flag this so
> hopefully someone else should double check this..... It would seem to
> me that the better thing to do would be to calculate the size as part
> of the for loop in ext3_acl_to_disk(), and drop ext3_acl_size() from
> acl.h. (This code exists in ext4 as well, so if we have a bug in
> ext3, we would have a similar bug in ext4.)
>
at least, for my idea:
your design for ext3_acl_size is a standard one.
it is necessary to use your design instead of original design.
if you also like me to provide the relative patch, please tell me.
thanks.
gchen.
>
> - Ted
>
>
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists