Re: [PATCH 1/3] xfs: ubsan fixes

[Christoph Hellwig <hch@infradead.org>]

On Tue, Nov 28, 2017 at 09:39:56AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Fix some complaints from the UBSAN about signed integer addition overflows.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I only noticed that now that it's in Linus' tree.  Need to find some
more time for the XFS list..

> -	if (offset + count > mp->m_super->s_maxbytes)
> +	if ((xfs_ufsize_t)offset + count > mp->m_super->s_maxbytes)
>  		count = mp->m_super->s_maxbytes - offset;

I don't think this fix is useful in any meaninfless way.  Yes,
signed overflow in C is undefined, and unsigned overflow is and this
will shut up UBSAN.  But it doesn't solve the problem at all.

Assuming we still need these checks and the VFS doesn't take care of
it already (I'd need to double check) we want to truncate at s_maxbytes,
and assuming s_maxbytes is close to ULLONG_MAX and count makes it
overflow this will give the wrong result, as it won't cap anything.

What we'd need instead would be something like:

	if (offset > mp->m_super->s_maxbytes - count)
		count = mp->m_super->s_maxbytes - offset;

as that does the right thing.