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