* 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