linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Chen Gang <gang.chen@asianux.com>
Cc: jack@suse.cz, akpm@linux-foundation.org, linux-ext4@vger.kernel.org
Subject: Re: [PATCH] fs/ext3: use kzalloc instead of kmalloc
Date: Tue, 25 Dec 2012 13:48:25 -0500	[thread overview]
Message-ID: <20121225184825.GD5318@thunk.org> (raw)
In-Reply-To: <50D7E815.6050503@asianux.com>

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@asianux.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.

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.

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.)


						- Ted

  reply	other threads:[~2012-12-25 18:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-24  5:28 [PATCH] fs/ext3: use kzalloc instead of kmalloc Chen Gang
2012-12-25 18:48 ` Theodore Ts'o [this message]
2012-12-26  3:15   ` Chen Gang
2012-12-26  4:45     ` Theodore Ts'o
2012-12-26  5:08       ` Chen Gang
2012-12-26  5:34         ` Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121225184825.GD5318@thunk.org \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=gang.chen@asianux.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).