* [PATCH] fs/ext3: set pointer = NULL, after kfree it
@ 2012-12-24 5:58 Chen Gang
2012-12-25 18:51 ` Theodore Ts'o
0 siblings, 1 reply; 4+ messages in thread
From: Chen Gang @ 2012-12-24 5:58 UTC (permalink / raw)
To: jack, akpm; +Cc: linux-ext4
set s->base = NULL, after kfree it.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
fs/ext3/xattr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
index d22ebb7..57aab94 100644
--- a/fs/ext3/xattr.c
+++ b/fs/ext3/xattr.c
@@ -847,8 +847,10 @@ cleanup:
if (ce)
mb_cache_entry_release(ce);
brelse(new_bh);
- if (!(bs->bh && s->base == bs->bh->b_data))
+ if (!(bs->bh && s->base == bs->bh->b_data)) {
kfree(s->base);
+ s->base = NULL;
+ }
return error;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it
2012-12-24 5:58 [PATCH] fs/ext3: set pointer = NULL, after kfree it Chen Gang
@ 2012-12-25 18:51 ` Theodore Ts'o
2012-12-26 2:33 ` Chen Gang
0 siblings, 1 reply; 4+ messages in thread
From: Theodore Ts'o @ 2012-12-25 18:51 UTC (permalink / raw)
To: Chen Gang; +Cc: jack, akpm, linux-ext4
On Mon, Dec 24, 2012 at 01:58:14PM +0800, Chen Gang wrote:
>
> set s->base = NULL, after kfree it.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
Was this to fix something flagged by some static code checker? The
only caller of ext3_xattr_block_set() is ext3_xattr_set_handle(), and
s->base is stored on the stack, and as soon as ext3_xattr_block_set()
returns, the object will disappear. So it seems pretty hard to see
how this could lead to a use-after-free bug.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it
2012-12-25 18:51 ` Theodore Ts'o
@ 2012-12-26 2:33 ` Chen Gang
2012-12-31 15:19 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Chen Gang @ 2012-12-26 2:33 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: jack, akpm, linux-ext4
于 2012年12月26日 02:51, Theodore Ts'o 写道:
> On Mon, Dec 24, 2012 at 01:58:14PM +0800, Chen Gang wrote:
>>
>> set s->base = NULL, after kfree it.
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>
> Was this to fix something flagged by some static code checker? The
> only caller of ext3_xattr_block_set() is ext3_xattr_set_handle(), and
> s->base is stored on the stack, and as soon as ext3_xattr_block_set()
> returns, the object will disappear. So it seems pretty hard to see
> how this could lead to a use-after-free bug.
>
> - Ted
>
>
this patch is not for a bug
it is only for coding habit:
it is better to set is to NULL, after kfree it.
in the future, if another functions need call ext3_xattr_block_set, too.
it will avoid another members to make mistake.
since it is not for fixing bug, it is really a minor patch.
I can understand if you do not apply it.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it
2012-12-26 2:33 ` Chen Gang
@ 2012-12-31 15:19 ` Jan Kara
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2012-12-31 15:19 UTC (permalink / raw)
To: Chen Gang; +Cc: Theodore Ts'o, jack, akpm, linux-ext4
On Wed 26-12-12 10:33:12, Chen Gang wrote:
> 于 2012年12月26日 02:51, Theodore Ts'o 写道:
> > On Mon, Dec 24, 2012 at 01:58:14PM +0800, Chen Gang wrote:
> >>
> >> set s->base = NULL, after kfree it.
> >>
> >> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> >
> > Was this to fix something flagged by some static code checker? The
> > only caller of ext3_xattr_block_set() is ext3_xattr_set_handle(), and
> > s->base is stored on the stack, and as soon as ext3_xattr_block_set()
> > returns, the object will disappear. So it seems pretty hard to see
> > how this could lead to a use-after-free bug.
> >
> > - Ted
> >
> >
>
> this patch is not for a bug
>
> it is only for coding habit:
> it is better to set is to NULL, after kfree it.
> in the future, if another functions need call ext3_xattr_block_set, too.
> it will avoid another members to make mistake.
Well, we set pointers to NULL after kfree() only if there's a good chance
we ever look at the pointer again. This is not the case so I don't think
this change makes the code any more safer / easier to read.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-12-31 15:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-24 5:58 [PATCH] fs/ext3: set pointer = NULL, after kfree it Chen Gang
2012-12-25 18:51 ` Theodore Ts'o
2012-12-26 2:33 ` Chen Gang
2012-12-31 15:19 ` Jan Kara
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).