re: xfs: use named array initializers for log item dumping

re: xfs: use named array initializers for log item dumping

[Dan Carpenter]

Hello Darrick J. Wong,

The patch 5110cd82ca90: "xfs: use named array initializers for log
item dumping" from Mar 7, 2016, leads to the following static checker
warning:

	fs/xfs/xfs_log.c:2085 xlog_print_tic_res()
	error: buffer overflow 'trans_type_str' 43 <= 43

fs/xfs/xfs_log.c
  2080  
  2081          xfs_warn(mp, "xlog_write: reservation summary:");
  2082          xfs_warn(mp, "  trans type  = %s (%u)",
  2083                   ((ticket->t_trans_type <= 0 ||
  2084                     ticket->t_trans_type > XFS_TRANS_TYPE_MAX) ?
                                                ^
Should be >=.  Why is zero invalid?

  2085                    "bad-trans-type" : trans_type_str[ticket->t_trans_type]),
  2086                   ticket->t_trans_type);
  2087          xfs_warn(mp, "  unit res    = %d bytes",


regards,
dan carpenter

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfs: use named array initializers for log item dumping

[Darrick J. Wong]

On Wed, Apr 06, 2016 at 01:56:57PM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
> 
> The patch 5110cd82ca90: "xfs: use named array initializers for log
> item dumping" from Mar 7, 2016, leads to the following static checker
> warning:
> 
> 	fs/xfs/xfs_log.c:2085 xlog_print_tic_res()
> 	error: buffer overflow 'trans_type_str' 43 <= 43
> 
> fs/xfs/xfs_log.c
>   2080  
>   2081          xfs_warn(mp, "xlog_write: reservation summary:");
>   2082          xfs_warn(mp, "  trans type  = %s (%u)",
>   2083                   ((ticket->t_trans_type <= 0 ||
>   2084                     ticket->t_trans_type > XFS_TRANS_TYPE_MAX) ?
>                                                 ^
> Should be >=.

Correct.  Good catch.

>  Why is zero invalid?

There isn't a XFS_TRANS_ code corresponding to zero:

/*
 * Transaction types.  Used to distinguish types of buffers. These never reach
 * the log.
 */
#define XFS_TRANS_SETATTR_NOT_SIZE	1
<etc>

That whole guard expression might as well be:

(ticket->t_trans_type == 0 || ticket->t_trans_type >= XFS_TRANS_TYPE_MAX)

Furthermore, XLOG_REG_TYPE_MAX could be 21 to be consistent with the rest
of XFS, and the checks for res_type_str usage below this could be the same.

(Though personally /me finds it odd that the _MAX values are usually one 
more than the last item in the list.)

(Also I thought there was other discussion of that patch so I'm a little
surprised to see it in mainline?)

--D

> 
>   2085                    "bad-trans-type" : trans_type_str[ticket->t_trans_type]),
>   2086                   ticket->t_trans_type);
>   2087          xfs_warn(mp, "  unit res    = %d bytes",
> 
> 
> regards,
> dan carpenter

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfs: use named array initializers for log item dumping

[Dave Chinner]

On Wed, Apr 06, 2016 at 01:56:57PM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
> 
> The patch 5110cd82ca90: "xfs: use named array initializers for log
> item dumping" from Mar 7, 2016, leads to the following static checker
> warning:
> 
> 	fs/xfs/xfs_log.c:2085 xlog_print_tic_res()
> 	error: buffer overflow 'trans_type_str' 43 <= 43
> 
> fs/xfs/xfs_log.c
>   2080  
>   2081          xfs_warn(mp, "xlog_write: reservation summary:");
>   2082          xfs_warn(mp, "  trans type  = %s (%u)",
>   2083                   ((ticket->t_trans_type <= 0 ||
>   2084                     ticket->t_trans_type > XFS_TRANS_TYPE_MAX) ?
>                                                 ^
> Should be >=.  Why is zero invalid?
> 
>   2085                    "bad-trans-type" : trans_type_str[ticket->t_trans_type]),
>   2086                   ticket->t_trans_type);
>   2087          xfs_warn(mp, "  unit res    = %d bytes",

I just pushed patches to the for-next branch (i.e. linux-next) that
remove this code.

https://git.kernel.org/cgit/linux/kernel/git/dgc/linux-xfs.git/commit/?h=for-next&id=710b1e2c2948c1e5d0499def5273ecbc6472342d

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs