Re: [PATCH] xfs: pin inodes that would otherwise overflow link count

[Dave Chinner <david@fromorbit.com>]

On Thu, Oct 12, 2023 at 08:08:20AM +1100, Dave Chinner wrote:
> On Wed, Oct 11, 2023 at 01:33:50PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > The VFS inc_nlink function does not explicitly check for integer
> > overflows in the i_nlink field.  Instead, it checks the link count
> > against s_max_links in the vfs_{link,create,rename} functions.  XFS
> > sets the maximum link count to 2.1 billion, so integer overflows should
> > not be a problem.
> > 
> > However.  It's possible that online repair could find that a file has
> > more than four billion links, particularly if the link count got
> 
> I don't think we should be attempting to fix that online - if we've
> really found an inode with 4 billion links then something else has
> gone wrong during repair because we shouldn't get there in the first
> place.
> 
> IOWs, we should be preventing a link count overflow at the time 
> that the link count is being added and returning -EMLINK errors to
> that operation. This prevents overflow, and so if repair does find
> more than 2.1 billion links to the inode, there's clearly something
> else very wrong (either in repair or a bug in the filesystem that
> has leaked many, many link counts).
> 
> huh.
> 
> We set sb->s_max_links = XFS_MAXLINKS, but nowhere does the VFS
> enforce that, nor does any XFS code. The lack of checking or
> enforcement of filesystem max link count anywhere is ... not ideal.

No, wait, I read the cscope output wrong. sb->s_max_links *is*
enforced at the VFS level, so we should never end up in a situation
with link count greater than XFS_MAXLINKS inside the XFS code in
normal operation. i.e. A count greater than that is an indication of
a software bug or corruption, so we should definitely be verifying
di_nlink is within the valid on-disk range regardless of anything
else....

-Dave.
-- 
Dave Chinner
david@fromorbit.com