On Sat, Mar 02, 2013 at 02:14:47PM -0600, Mark Tinguely wrote:
> When the iclog buffer size and log stripe unit are both defined and
> the log stripe unit is less the log buffer size then the buffer is
> rounded up to the log stripe unit size during the xlog_sync().
>
> This rounding can exceed the iclog buffer length and in xlog_data_pack():
> 1) Cause corruption inside the iclog buffer because there will not be
> enough space for the headers in the front of the iclog buffer for
> the rounding.
> 2) Cause corruption in memory that follows the iclog buffer when
> stamping the lsn in each of the rounded blocks.
> 3) If CONFIG_XFS_DEBUG is defined will cause a crash in xlog_verify_iclog().
> 4) Cause page fault crash if the memory after the buffer is not mapped.
>
> This has been found in XFS versions at least as far back as
> Linux 2.6.32.
>
> This patch forces the iclog buffer to be a multiple of the log stripe
> unit when they are both defined.
>
> Example:
> # mkfs.xfs -l su=192k -f /dev/sda2
$ sudo mkfs.xfs -l sunit=192k /dev/vdb
Specify log sunit in 512-byte blocks, no size suffix
....
$
> # mount -o logbsize=256k /dev/sda3 /scratch
> # io such as fsstress in /scratch will immediately crash a debug xfs
> kernel and most like a non-debug xfs kernel.
It's definitely not an immediate crash. xfstests runs for several
minutes (including through fsstress workloads) before it finally
falls over. I'd suggest a targetted xfstest is needed for this...
> Index: b/fs/xfs/xfs_super.c
> ===================================================================
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1357,11 +1357,23 @@ xfs_finish_flags(
> if (mp->m_logbsize <= 0 &&
> mp->m_sb.sb_logsunit > XLOG_BIG_RECORD_BSIZE) {
> mp->m_logbsize = mp->m_sb.sb_logsunit;
> - } else if (mp->m_logbsize > 0 &&
> - mp->m_logbsize < mp->m_sb.sb_logsunit) {
> - xfs_warn(mp,
> + } else if (mp->m_logbsize > 0) {
> + if (mp->m_logbsize > mp->m_sb.sb_logsunit) {
> + int logbsize;
> + /* round up to the next multiple of logsunit */
> + logbsize = roundup(mp->m_logbsize,
> + mp->m_sb.sb_logsunit);
> + if (logbsize > XLOG_MAX_RECORD_BSIZE)
> + /* buffer size too large. round down. */
> + logbsize -= mp->m_sb.sb_logsunit;
> + xfs_warn(mp, "log bufsize rounded from %d to %d",
> + mp->m_logbsize, logbsize);
> + mp->m_logbsize = logbsize;
> + } else if (mp->m_logbsize < mp->m_sb.sb_logsunit) {
> + xfs_warn(mp,
> "logbuf size must be greater than or equal to log stripe size");
> - return XFS_ERROR(EINVAL);
> + return XFS_ERROR(EINVAL);
> + }
If the user has specified an invalid log buffer size, then reject it
with:
logbsize XXX is not an integer multiple of the log stripe unit YYY
Rounding means that the user isn't getting what they want and they
may not realise it. If they make a mistake, they should be informed
and forced to fix it before going any further.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
