Skip to contentSkip to conversation

xfs: use named array initializers for log item dumping

DISCUSSION Dan Carpenter 3 participants 3 messages Apr 6, 2016

lore
Share & tools
lore
#0 lore raw

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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 9 lines · 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
Quoted 8 lines

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.

Quoted 1 line

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

Quoted 8 lines

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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw
In reply to Dan Carpenter
Quoted 15 lines · 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) ?

Quoted 6 lines
                                                ^

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.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs