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
corrupted while creating hardlinks to the file. The di_nlinkv2 field is
not large enough to store a value larger than 2^32, so we ought to
define a magic pin value of ~0U which means that the inode never gets
deleted. This will prevent a UAF error if the repair finds this
situation and users begin deleting links to the file.
Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_format.h | 6 ++++++
fs/xfs/scrub/nlinks.c | 8 ++++----
fs/xfs/scrub/repair.c | 12 ++++++------
fs/xfs/xfs_inode.c | 16 ++++++++++++----
4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 0c457905cce5..977d30519738 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -891,6 +891,12 @@ static inline uint xfs_dinode_size(int version)
*/
#define XFS_MAXLINK ((1U << 31) - 1U)
+/*
+ * Any file that hits the maximum ondisk link count should be pinned to avoid
+ * a use-after-free situation.
+ */
+#define XFS_NLINK_PINNED (~0U)
+
/*
* Values for di_format
*
diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
index 67e2c167fc36..0507b80458d1 100644
--- a/fs/xfs/scrub/nlinks.c
+++ b/fs/xfs/scrub/nlinks.c
@@ -93,8 +93,8 @@ xchk_setup_nlinks(
/*
* Add a delta to an nlink counter, being careful about integer overflow.
- * Clamp the value to U32_MAX because the ondisk format does not handle
- * link counts any higher.
+ * Clamp the value to XFS_NLINK_PINNED because the ondisk format does not
+ * handle link counts any higher.
*/
static inline void
careful_add(
@@ -103,7 +103,7 @@ careful_add(
{
uint64_t new_value = (uint64_t)(*nlinkp) + delta;
- *nlinkp = min_t(uint64_t, new_value, U32_MAX);
+ *nlinkp = min_t(uint64_t, new_value, XFS_NLINK_PINNED);
}
/* Update incore link count information. Caller must hold the nlinks lock. */
@@ -602,7 +602,7 @@ xchk_nlinks_compare_inode(
* this. The VFS won't let users increase the link count, but it will
* let them decrease it.
*/
- if (total_links > U32_MAX)
+ if (total_links > XFS_NLINK_PINNED)
xchk_ino_set_corrupt(sc, ip->i_ino);
else if (total_links > XFS_MAXLINK)
xchk_ino_set_warning(sc, ip->i_ino);
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index f42ce1995caa..042bd32f5799 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1163,15 +1163,15 @@ xrep_set_nlink(
{
bool ret = false;
- if (nlink > U32_MAX) {
+ if (nlink > XFS_NLINK_PINNED) {
/*
* The observed link count will overflow the nlink field.
*
* The VFS won't let users create more hardlinks if the link
* count is larger than XFS_MAXLINK, but it will let them
- * delete hardlinks. XFS_MAXLINK is half of U32_MAX, which
- * means that sysadmins could actually fix this situation by
- * deleting links and calling us again.
+ * delete hardlinks. XFS_MAXLINK is half of XFS_NLINK_PINNED,
+ * which means that sysadmins could actually fix this situation
+ * by deleting links and calling us again.
*
* Set the link count to the largest possible value that will
* fit in the field. This will buy us the most possible time
@@ -1179,9 +1179,9 @@ xrep_set_nlink(
* As long as the link count stays above MAXLINK the undercount
* problem will not get worse.
*/
- BUILD_BUG_ON((uint64_t)XFS_MAXLINK >= U32_MAX);
+ BUILD_BUG_ON((uint64_t)XFS_MAXLINK >= XFS_NLINK_PINNED);
- nlink = U32_MAX;
+ nlink = XFS_NLINK_PINNED;
ret = true;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index f7dfd8e50583..2edc52e747f2 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -911,12 +911,16 @@ xfs_init_new_inode(
*/
static int /* error */
xfs_droplink(
- xfs_trans_t *tp,
- xfs_inode_t *ip)
+ struct xfs_trans *tp,
+ struct xfs_inode *ip)
{
+ struct inode *inode = VFS_I(ip);
+
xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
- drop_nlink(VFS_I(ip));
+ if (inode->i_nlink != XFS_NLINK_PINNED)
+ drop_nlink(VFS_I(ip));
+
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
if (VFS_I(ip)->i_nlink)
@@ -933,9 +937,13 @@ xfs_bumplink(
struct xfs_trans *tp,
struct xfs_inode *ip)
{
+ struct inode *inode = VFS_I(ip);
+
xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
- inc_nlink(VFS_I(ip));
+ if (inode->i_nlink != XFS_NLINK_PINNED)
+ inc_nlink(VFS_I(ip));
+
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
}
