[PATCH] fs/ext3: set pointer = NULL, after kfree it

[PATCH] fs/ext3: set pointer = NULL, after kfree it

[Chen Gang]

  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

Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it

[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

Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it

[Chen Gang]

于 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

Re: [PATCH] fs/ext3: set pointer = NULL, after kfree it

[Jan Kara]

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