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

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Oct 12, 2023 at 08:14:44AM +1100, Dave Chinner wrote:
> 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....

... and I just realized that the VFS doesn't check for underflows when
unlinking or rmdir'ing.  Maybe it should be doing that instead of
patching XFS and everything else?

--D

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