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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxx
