From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org, aalbersh@redhat.com, ebiggers@kernel.org
Cc: Allison Henderson <allison.henderson@oracle.com>,
linux-fsdevel@vger.kernel.org, fsverity@lists.linux.dev,
linux-xfs@vger.kernel.org
Subject: [PATCH 04/29] xfs: add parent pointer validator functions
Date: Wed, 13 Mar 2024 10:53:37 -0700 [thread overview]
Message-ID: <171035223424.2613863.10249525423420622979.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <171035223299.2613863.12196197862413309469.stgit@frogsfrogsfrogs>
From: Allison Henderson <allison.henderson@oracle.com>
Attribute names of parent pointers are not strings. So we need to
modify attr_namecheck to verify parent pointer records when the
XFS_ATTR_PARENT flag is set. At the same time, we need to validate attr
values during log recovery if the xattr is really a parent pointer.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
[djwong: move functions to xfs_parent.c, adjust for new disk format]
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/Makefile | 1
fs/xfs/libxfs/xfs_attr.c | 10 +++-
fs/xfs/libxfs/xfs_attr.h | 3 +
fs/xfs/libxfs/xfs_da_format.h | 8 +++
fs/xfs/libxfs/xfs_parent.c | 113 +++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_parent.h | 19 +++++++
fs/xfs/scrub/attr.c | 2 -
fs/xfs/xfs_attr_item.c | 6 +-
fs/xfs/xfs_attr_list.c | 14 +++--
9 files changed, 165 insertions(+), 11 deletions(-)
create mode 100644 fs/xfs/libxfs/xfs_parent.c
create mode 100644 fs/xfs/libxfs/xfs_parent.h
diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index 76674ad5833e..f8845e65cac7 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -41,6 +41,7 @@ xfs-y += $(addprefix libxfs/, \
xfs_inode_buf.o \
xfs_log_rlimit.o \
xfs_ag_resv.o \
+ xfs_parent.o \
xfs_rmap.o \
xfs_rmap_btree.o \
xfs_refcount.o \
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index ff67a684a452..f0b625d45aa4 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -26,6 +26,7 @@
#include "xfs_trace.h"
#include "xfs_attr_item.h"
#include "xfs_xattr.h"
+#include "xfs_parent.h"
struct kmem_cache *xfs_attr_intent_cache;
@@ -1515,9 +1516,14 @@ xfs_attr_node_get(
/* Returns true if the attribute entry name is valid. */
bool
xfs_attr_namecheck(
- const void *name,
- size_t length)
+ struct xfs_mount *mp,
+ const void *name,
+ size_t length,
+ unsigned int flags)
{
+ if (flags & XFS_ATTR_PARENT)
+ return xfs_parent_namecheck(mp, name, length, flags);
+
/*
* MAXNAMELEN includes the trailing null, but (name/length) leave it
* out, so use >= for the length check.
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 81be9b3e4004..92711c8d2a9f 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -547,7 +547,8 @@ int xfs_attr_get(struct xfs_da_args *args);
int xfs_attr_set(struct xfs_da_args *args);
int xfs_attr_set_iter(struct xfs_attr_intent *attr);
int xfs_attr_remove_iter(struct xfs_attr_intent *attr);
-bool xfs_attr_namecheck(const void *name, size_t length);
+bool xfs_attr_namecheck(struct xfs_mount *mp, const void *name, size_t length,
+ unsigned int flags);
int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
void xfs_init_attr_trans(struct xfs_da_args *args, struct xfs_trans_res *tres,
unsigned int *total);
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 67e8c33c4e82..839df0e5401b 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -757,6 +757,14 @@ xfs_attr3_leaf_name(xfs_attr_leafblock_t *leafp, int idx)
return &((char *)leafp)[be16_to_cpu(entries[idx].nameidx)];
}
+static inline int
+xfs_attr3_leaf_flags(xfs_attr_leafblock_t *leafp, int idx)
+{
+ struct xfs_attr_leaf_entry *entries = xfs_attr3_leaf_entryp(leafp);
+
+ return entries[idx].flags;
+}
+
static inline xfs_attr_leaf_name_remote_t *
xfs_attr3_leaf_name_remote(xfs_attr_leafblock_t *leafp, int idx)
{
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
new file mode 100644
index 000000000000..1d45f926c13a
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022-2024 Oracle.
+ * All rights reserved.
+ */
+#include "xfs.h"
+#include "xfs_fs.h"
+#include "xfs_format.h"
+#include "xfs_da_format.h"
+#include "xfs_log_format.h"
+#include "xfs_shared.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_bmap_btree.h"
+#include "xfs_inode.h"
+#include "xfs_error.h"
+#include "xfs_trace.h"
+#include "xfs_trans.h"
+#include "xfs_da_btree.h"
+#include "xfs_attr.h"
+#include "xfs_dir2.h"
+#include "xfs_dir2_priv.h"
+#include "xfs_attr_sf.h"
+#include "xfs_bmap.h"
+#include "xfs_defer.h"
+#include "xfs_log.h"
+#include "xfs_xattr.h"
+#include "xfs_parent.h"
+#include "xfs_trans_space.h"
+
+/*
+ * Parent pointer attribute handling.
+ *
+ * Because the attribute value is a filename component, it will never be longer
+ * than 255 bytes. This means the attribute will always be a local format
+ * attribute as it is xfs_attr_leaf_entsize_local_max() for v5 filesystems will
+ * always be larger than this (max is 75% of block size).
+ *
+ * Creating a new parent attribute will always create a new attribute - there
+ * should never, ever be an existing attribute in the tree for a new inode.
+ * ENOSPC behavior is problematic - creating the inode without the parent
+ * pointer is effectively a corruption, so we allow parent attribute creation
+ * to dip into the reserve block pool to avoid unexpected ENOSPC errors from
+ * occurring.
+ */
+
+/* Return true if parent pointer EA name is valid. */
+bool
+xfs_parent_namecheck(
+ struct xfs_mount *mp,
+ const struct xfs_parent_name_rec *rec,
+ size_t reclen,
+ unsigned int attr_flags)
+{
+ if (!(attr_flags & XFS_ATTR_PARENT))
+ return false;
+
+ /* pptr updates use logged xattrs, so we should never see this flag */
+ if (attr_flags & XFS_ATTR_INCOMPLETE)
+ return false;
+
+ if (reclen != sizeof(struct xfs_parent_name_rec))
+ return false;
+
+ /* Only one namespace bit allowed. */
+ if (hweight32(attr_flags & XFS_ATTR_NSP_ONDISK_MASK) > 1)
+ return false;
+
+ return true;
+}
+
+/* Return true if parent pointer EA value is valid. */
+bool
+xfs_parent_valuecheck(
+ struct xfs_mount *mp,
+ const void *value,
+ size_t valuelen)
+{
+ if (valuelen == 0 || valuelen > XFS_PARENT_DIRENT_NAME_MAX_SIZE)
+ return false;
+
+ if (value == NULL)
+ return false;
+
+ return true;
+}
+
+/* Return true if the ondisk parent pointer is consistent. */
+bool
+xfs_parent_hashcheck(
+ struct xfs_mount *mp,
+ const struct xfs_parent_name_rec *rec,
+ const void *value,
+ size_t valuelen)
+{
+ struct xfs_name dname = {
+ .name = value,
+ .len = valuelen,
+ };
+ xfs_ino_t p_ino;
+
+ /* Valid dirent name? */
+ if (!xfs_dir2_namecheck(value, valuelen))
+ return false;
+
+ /* Valid inode number? */
+ p_ino = be64_to_cpu(rec->p_ino);
+ if (!xfs_verify_dir_ino(mp, p_ino))
+ return false;
+
+ /* Namehash matches name? */
+ return be32_to_cpu(rec->p_namehash) == xfs_dir2_hashname(mp, &dname);
+}
diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h
new file mode 100644
index 000000000000..fcfeddb645f6
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_parent.h
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022-2024 Oracle.
+ * All Rights Reserved.
+ */
+#ifndef __XFS_PARENT_H__
+#define __XFS_PARENT_H__
+
+/* Metadata validators */
+bool xfs_parent_namecheck(struct xfs_mount *mp,
+ const struct xfs_parent_name_rec *rec, size_t reclen,
+ unsigned int attr_flags);
+bool xfs_parent_valuecheck(struct xfs_mount *mp, const void *value,
+ size_t valuelen);
+bool xfs_parent_hashcheck(struct xfs_mount *mp,
+ const struct xfs_parent_name_rec *rec, const void *value,
+ size_t valuelen);
+
+#endif /* __XFS_PARENT_H__ */
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 49f91cc85a65..9a1f59f7b5a4 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -195,7 +195,7 @@ xchk_xattr_listent(
}
/* Does this name make sense? */
- if (!xfs_attr_namecheck(name, namelen)) {
+ if (!xfs_attr_namecheck(sx->sc->mp, name, namelen, flags)) {
xchk_fblock_set_corrupt(sx->sc, XFS_ATTR_FORK, args.blkno);
goto fail_xref;
}
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 9b4c61e1c22e..703770cf1482 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -591,7 +591,8 @@ xfs_attr_recover_work(
*/
attrp = &attrip->attri_format;
if (!xfs_attri_validate(mp, attrp) ||
- !xfs_attr_namecheck(nv->name.i_addr, nv->name.i_len))
+ !xfs_attr_namecheck(mp, nv->name.i_addr, nv->name.i_len,
+ attrp->alfi_attr_filter))
return -EFSCORRUPTED;
attr = xfs_attri_recover_work(mp, dfp, attrp, &ip, nv);
@@ -731,7 +732,8 @@ xlog_recover_attri_commit_pass2(
return -EFSCORRUPTED;
}
- if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
+ if (!xfs_attr_namecheck(mp, attr_name, attri_formatp->alfi_name_len,
+ attri_formatp->alfi_attr_filter)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
item->ri_buf[1].i_addr, item->ri_buf[1].i_len);
return -EFSCORRUPTED;
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index a6819a642cc0..fa74378577c5 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -59,6 +59,7 @@ xfs_attr_shortform_list(
struct xfs_attr_sf_sort *sbuf, *sbp;
struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
+ struct xfs_mount *mp = dp->i_mount;
int sbsize, nsbuf, count, i;
int error = 0;
@@ -82,8 +83,9 @@ xfs_attr_shortform_list(
(dp->i_af.if_bytes + sf->count * 16) < context->bufsize)) {
for (i = 0, sfe = xfs_attr_sf_firstentry(sf); i < sf->count; i++) {
if (XFS_IS_CORRUPT(context->dp->i_mount,
- !xfs_attr_namecheck(sfe->nameval,
- sfe->namelen))) {
+ !xfs_attr_namecheck(mp, sfe->nameval,
+ sfe->namelen,
+ sfe->flags))) {
xfs_dirattr_mark_sick(context->dp, XFS_ATTR_FORK);
return -EFSCORRUPTED;
}
@@ -177,8 +179,9 @@ xfs_attr_shortform_list(
cursor->offset = 0;
}
if (XFS_IS_CORRUPT(context->dp->i_mount,
- !xfs_attr_namecheck(sbp->name,
- sbp->namelen))) {
+ !xfs_attr_namecheck(mp, sbp->name,
+ sbp->namelen,
+ sbp->flags))) {
xfs_dirattr_mark_sick(context->dp, XFS_ATTR_FORK);
error = -EFSCORRUPTED;
goto out;
@@ -474,7 +477,8 @@ xfs_attr3_leaf_list_int(
}
if (XFS_IS_CORRUPT(context->dp->i_mount,
- !xfs_attr_namecheck(name, namelen))) {
+ !xfs_attr_namecheck(mp, name, namelen,
+ entry->flags))) {
xfs_dirattr_mark_sick(context->dp, XFS_ATTR_FORK);
return -EFSCORRUPTED;
}
next prev parent reply other threads:[~2024-03-13 17:53 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-13 17:52 [PATCHSET v5.2] fs-verity support for XFS Darrick J. Wong
2024-03-13 17:52 ` [PATCH 01/29] fsverity: remove hash page spin lock Darrick J. Wong
2024-03-13 17:53 ` [PATCH 02/29] xfs: add parent pointer support to attribute code Darrick J. Wong
2024-03-13 17:53 ` [PATCH 03/29] xfs: define parent pointer ondisk extended attribute format Darrick J. Wong
2024-03-13 17:53 ` Darrick J. Wong [this message]
2024-03-13 17:53 ` [PATCH 05/29] fs: add FS_XFLAG_VERITY for verity files Darrick J. Wong
2024-03-13 17:54 ` [PATCH 06/29] fsverity: pass tree_blocksize to end_enable_verity() Darrick J. Wong
2024-03-13 17:54 ` [PATCH 07/29] fsverity: support block-based Merkle tree caching Darrick J. Wong
2024-03-13 17:54 ` [PATCH 08/29] fsverity: add per-sb workqueue for post read processing Darrick J. Wong
2024-03-19 23:30 ` Darrick J. Wong
2024-03-20 10:37 ` Andrey Albershteyn
2024-03-20 14:55 ` Darrick J. Wong
2024-03-20 16:22 ` Andrey Albershteyn
2024-03-13 17:54 ` [PATCH 09/29] fsverity: add tracepoints Darrick J. Wong
2024-03-13 17:55 ` [PATCH 10/29] fsverity: fix "support block-based Merkle tree caching" Darrick J. Wong
2024-03-13 17:55 ` [PATCH 11/29] fsverity: send the level of the merkle tree block to ->read_merkle_tree_block Darrick J. Wong
2024-03-13 17:55 ` [PATCH 12/29] fsverity: pass the new tree size and block size to ->begin_enable_verity Darrick J. Wong
2024-03-13 17:55 ` [PATCH 13/29] iomap: integrate fs-verity verification into iomap's read path Darrick J. Wong
2024-03-13 17:56 ` [PATCH 14/29] xfs: add attribute type for fs-verity Darrick J. Wong
2024-03-13 17:56 ` [PATCH 15/29] xfs: add fs-verity ro-compat flag Darrick J. Wong
2024-03-13 17:56 ` [PATCH 16/29] xfs: add inode on-disk VERITY flag Darrick J. Wong
2024-03-13 17:57 ` [PATCH 17/29] xfs: initialize fs-verity on file open and cleanup on inode destruction Darrick J. Wong
2024-03-13 17:57 ` [PATCH 18/29] xfs: don't allow to enable DAX on fs-verity sealed inode Darrick J. Wong
2024-03-13 17:57 ` [PATCH 19/29] xfs: disable direct read path for fs-verity files Darrick J. Wong
2024-03-13 17:57 ` [PATCH 20/29] xfs: widen flags argument to the xfs_iflags_* helpers Darrick J. Wong
2024-03-13 17:58 ` [PATCH 21/29] xfs: add fs-verity support Darrick J. Wong
2024-03-14 17:06 ` Darrick J. Wong
2024-03-14 17:16 ` Andrey Albershteyn
2024-03-15 2:59 ` Darrick J. Wong
2024-03-13 17:58 ` [PATCH 22/29] xfs: create a per-mount shrinker for verity inodes merkle tree blocks Darrick J. Wong
2024-03-13 17:58 ` [PATCH 23/29] xfs: create an icache tag for files with cached " Darrick J. Wong
2024-03-13 17:58 ` [PATCH 24/29] xfs: shrink verity blob cache Darrick J. Wong
2024-03-13 17:59 ` [PATCH 25/29] xfs: clean up stale fsverity metadata before starting Darrick J. Wong
2024-03-13 17:59 ` [PATCH 26/29] xfs: better reporting and error handling in xfs_drop_merkle_tree Darrick J. Wong
2024-03-13 17:59 ` [PATCH 27/29] xfs: make scrub aware of verity dinode flag Darrick J. Wong
2024-03-13 17:59 ` [PATCH 28/29] xfs: add fs-verity ioctls Darrick J. Wong
2024-03-13 18:00 ` [PATCH 29/29] xfs: enable ro-compat fs-verity flag Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=171035223424.2613863.10249525423420622979.stgit@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=aalbersh@redhat.com \
--cc=allison.henderson@oracle.com \
--cc=ebiggers@kernel.org \
--cc=fsverity@lists.linux.dev \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).