* REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c
@ 2008-06-02 5:42 Barry Naujok
2008-06-02 5:50 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Barry Naujok @ 2008-06-02 5:42 UTC (permalink / raw)
To: xfs@oss.sgi.com
In particular, this patch fixes a problem in the xfs_dir2_remove and
xfs_dir2_replace paths which internally can call a lookup function
which will use args->cmpresult which is uninitialised.
--
Index: 2.6.x-xfs/fs/xfs/xfs_dir2.c
===================================================================
--- 2.6.x-xfs.orig/fs/xfs/xfs_dir2.c
+++ 2.6.x-xfs/fs/xfs/xfs_dir2.c
@@ -213,6 +213,7 @@ xfs_dir_createname(
if ((rval = xfs_dir_ino_validate(tp->t_mountp, inum)))
return rval;
XFS_STATS_INC(xs_dir_create);
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
@@ -297,7 +298,6 @@ xfs_dir_lookup(
args.op_flags = XFS_DA_OP_OKNOENT;
if (ci_name)
args.op_flags |= XFS_DA_OP_CILOOKUP;
- args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_lookup(&args);
@@ -342,6 +342,7 @@ xfs_dir_removename(
ASSERT((dp->i_d.di_mode & S_IFMT) == S_IFDIR);
XFS_STATS_INC(xs_dir_remove);
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
@@ -353,7 +354,6 @@ xfs_dir_removename(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_removename(&args);
@@ -425,6 +425,7 @@ xfs_dir_replace(
if ((rval = xfs_dir_ino_validate(tp->t_mountp, inum)))
return rval;
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
@@ -436,7 +437,6 @@ xfs_dir_replace(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_replace(&args);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c
2008-06-02 5:42 REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c Barry Naujok
@ 2008-06-02 5:50 ` Christoph Hellwig
2008-06-02 6:08 ` Barry Naujok
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2008-06-02 5:50 UTC (permalink / raw)
To: Barry Naujok; +Cc: xfs@oss.sgi.com
On Mon, Jun 02, 2008 at 03:42:55PM +1000, Barry Naujok wrote:
> In particular, this patch fixes a problem in the xfs_dir2_remove and
> xfs_dir2_replace paths which internally can call a lookup function
> which will use args->cmpresult which is uninitialised.
> Index: 2.6.x-xfs/fs/xfs/xfs_dir2.c
> ===================================================================
> --- 2.6.x-xfs.orig/fs/xfs/xfs_dir2.c
> +++ 2.6.x-xfs/fs/xfs/xfs_dir2.c
> @@ -213,6 +213,7 @@ xfs_dir_createname(
> if ((rval = xfs_dir_ino_validate(tp->t_mountp, inum)))
> return rval;
> XFS_STATS_INC(xs_dir_create);
> + memset(&args, 0, sizeof(xfs_da_args_t));
>
> args.name = name->name;
> args.namelen = name->len;
Doing these memsets looks good. Stylisticly I'd rather put them
directly in front of the intialization for the actually used args
fields.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c
2008-06-02 5:50 ` Christoph Hellwig
@ 2008-06-02 6:08 ` Barry Naujok
2008-06-02 6:08 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Barry Naujok @ 2008-06-02 6:08 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs@oss.sgi.com
On Mon, 02 Jun 2008 15:50:28 +1000, Christoph Hellwig <hch@infradead.org>
wrote:
> On Mon, Jun 02, 2008 at 03:42:55PM +1000, Barry Naujok wrote:
>> In particular, this patch fixes a problem in the xfs_dir2_remove and
>> xfs_dir2_replace paths which internally can call a lookup function
>> which will use args->cmpresult which is uninitialised.
>
>> Index: 2.6.x-xfs/fs/xfs/xfs_dir2.c
>> ===================================================================
>> --- 2.6.x-xfs.orig/fs/xfs/xfs_dir2.c
>> +++ 2.6.x-xfs/fs/xfs/xfs_dir2.c
>> @@ -213,6 +213,7 @@ xfs_dir_createname(
>> if ((rval = xfs_dir_ino_validate(tp->t_mountp, inum)))
>> return rval;
>> XFS_STATS_INC(xs_dir_create);
>> + memset(&args, 0, sizeof(xfs_da_args_t));
>>
>> args.name = name->name;
>> args.namelen = name->len;
>
> Doing these memsets looks good. Stylisticly I'd rather put them
> directly in front of the intialization for the actually used args
> fields.
I agree, so here's the alternative patch which makes them all
consistent:
Index: 2.6.x-xfs/fs/xfs/xfs_dir2.c
===================================================================
--- 2.6.x-xfs.orig/fs/xfs/xfs_dir2.c
+++ 2.6.x-xfs/fs/xfs/xfs_dir2.c
@@ -214,6 +214,7 @@ xfs_dir_createname(
return rval;
XFS_STATS_INC(xs_dir_create);
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
args.hashval = dp->i_mount->m_dirnameops->hashname(name);
@@ -286,8 +287,8 @@ xfs_dir_lookup(
ASSERT((dp->i_d.di_mode & S_IFMT) == S_IFDIR);
XFS_STATS_INC(xs_dir_lookup);
- memset(&args, 0, sizeof(xfs_da_args_t));
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
args.hashval = dp->i_mount->m_dirnameops->hashname(name);
@@ -297,7 +298,6 @@ xfs_dir_lookup(
args.op_flags = XFS_DA_OP_OKNOENT;
if (ci_name)
args.op_flags |= XFS_DA_OP_CILOOKUP;
- args.cmpresult = XFS_CMP_DIFFERENT;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_lookup(&args);
@@ -343,6 +343,7 @@ xfs_dir_removename(
ASSERT((dp->i_d.di_mode & S_IFMT) == S_IFDIR);
XFS_STATS_INC(xs_dir_remove);
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
args.hashval = dp->i_mount->m_dirnameops->hashname(name);
@@ -353,7 +354,6 @@ xfs_dir_removename(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_removename(&args);
@@ -426,6 +426,7 @@ xfs_dir_replace(
if ((rval = xfs_dir_ino_validate(tp->t_mountp, inum)))
return rval;
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
args.hashval = dp->i_mount->m_dirnameops->hashname(name);
@@ -436,7 +437,6 @@ xfs_dir_replace(
args.total = total;
args.whichfork = XFS_DATA_FORK;
args.trans = tp;
- args.op_flags = 0;
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_replace(&args);
@@ -472,8 +472,8 @@ xfs_dir_canenter(
return 0;
ASSERT((dp->i_d.di_mode & S_IFMT) == S_IFDIR);
- memset(&args, 0, sizeof(xfs_da_args_t));
+ memset(&args, 0, sizeof(xfs_da_args_t));
args.name = name->name;
args.namelen = name->len;
args.hashval = dp->i_mount->m_dirnameops->hashname(name);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c
2008-06-02 6:08 ` Barry Naujok
@ 2008-06-02 6:08 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2008-06-02 6:08 UTC (permalink / raw)
To: Barry Naujok; +Cc: Christoph Hellwig, xfs@oss.sgi.com
On Mon, Jun 02, 2008 at 04:08:23PM +1000, Barry Naujok wrote:
> I agree, so here's the alternative patch which makes them all
> consistent:
Looks good.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-06-02 6:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-02 5:42 REVIEW: Zero uninitialised xfs_da_args structure in xfs_dir2.c Barry Naujok
2008-06-02 5:50 ` Christoph Hellwig
2008-06-02 6:08 ` Barry Naujok
2008-06-02 6:08 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox