Commit 60382993 authored by Darrick J. Wong's avatar Darrick J. Wong
Browse files

xfs: get rid of the xchk_xfile_*_descr calls 



The xchk_xfile_*_descr macros call kasprintf, which can fail to allocate
memory if the formatted string is larger than 16 bytes (or whatever the
nofail guarantees are nowadays).  Some of them could easily exceed that,
and Jiaming Zhang found a few places where that can happen with syzbot.

The descriptions are debugging aids and aren't required to be unique, so
let's just pass in static strings and eliminate this path to failure.
Note this patch touches a number of commits, most of which were merged
between 6.6 and 6.14.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.12
Fixes: ab97f4b1 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: default avatar"Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Tested-by: default avatarJiaming Zhang <r772577952@gmail.com>
parent eaec8aef

fs/xfs/scrub/agheader_repair.c

+4 −9
Original line number Diff line number Diff line
@@ -1708,7 +1708,6 @@ xrep_agi( 
{

	struct xrep_agi		*ragi;

	struct xfs_mount	*mp = sc->mp;

	char			*descr;

	unsigned int		i;

	int			error;
@@ -1742,17 +1741,13 @@ xrep_agi( 
	xagino_bitmap_init(&ragi->iunlink_bmp);

	sc->buf_cleanup = xrep_agi_buf_cleanup;



	descr = xchk_xfile_ag_descr(sc, "iunlinked next pointers");

	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),

			&ragi->iunlink_next);

	kfree(descr);

	error = xfarray_create("iunlinked next pointers", 0,

			sizeof(xfs_agino_t), &ragi->iunlink_next);

	if (error)

		return error;



	descr = xchk_xfile_ag_descr(sc, "iunlinked prev pointers");

	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),

			&ragi->iunlink_prev);

	kfree(descr);

	error = xfarray_create("iunlinked prev pointers", 0,

			sizeof(xfs_agino_t), &ragi->iunlink_prev);

	if (error)

		return error;

fs/xfs/scrub/alloc_repair.c

+1 −4
Original line number Diff line number Diff line
@@ -850,7 +850,6 @@ xrep_allocbt( 
	struct xrep_abt		*ra;

	struct xfs_mount	*mp = sc->mp;

	unsigned int		busy_gen;

	char			*descr;

	int			error;



	/* We require the rmapbt to rebuild anything. */
@@ -876,11 +875,9 @@ xrep_allocbt( 
	}



	/* Set up enough storage to handle maximally fragmented free space. */

	descr = xchk_xfile_ag_descr(sc, "free space records");

	error = xfarray_create(descr, mp->m_sb.sb_agblocks / 2,

	error = xfarray_create("free space records", mp->m_sb.sb_agblocks / 2,

			sizeof(struct xfs_alloc_rec_incore),

			&ra->free_records);

	kfree(descr);

	if (error)

		goto out_ra;

fs/xfs/scrub/attr_repair.c

+5 −15
Original line number Diff line number Diff line
@@ -1529,7 +1529,6 @@ xrep_xattr_setup_scan( 
	struct xrep_xattr	**rxp)

{

	struct xrep_xattr	*rx;

	char			*descr;

	int			max_len;

	int			error;
@@ -1555,35 +1554,26 @@ xrep_xattr_setup_scan( 
		goto out_rx;



	/* Set up some staging for salvaged attribute keys and values */

	descr = xchk_xfile_ino_descr(sc, "xattr keys");

	error = xfarray_create(descr, 0, sizeof(struct xrep_xattr_key),

	error = xfarray_create("xattr keys", 0, sizeof(struct xrep_xattr_key),

			&rx->xattr_records);

	kfree(descr);

	if (error)

		goto out_rx;



	descr = xchk_xfile_ino_descr(sc, "xattr names");

	error = xfblob_create(descr, &rx->xattr_blobs);

	kfree(descr);

	error = xfblob_create("xattr names", &rx->xattr_blobs);

	if (error)

		goto out_keys;



	if (xfs_has_parent(sc->mp)) {

		ASSERT(sc->flags & XCHK_FSGATES_DIRENTS);



		descr = xchk_xfile_ino_descr(sc,

				"xattr retained parent pointer entries");

		error = xfarray_create(descr, 0,

		error = xfarray_create("xattr parent pointer entries", 0,

				sizeof(struct xrep_xattr_pptr),

				&rx->pptr_recs);

		kfree(descr);

		if (error)

			goto out_values;



		descr = xchk_xfile_ino_descr(sc,

				"xattr retained parent pointer names");

		error = xfblob_create(descr, &rx->pptr_names);

		kfree(descr);

		error = xfblob_create("xattr parent pointer names",

				&rx->pptr_names);

		if (error)

			goto out_pprecs;

fs/xfs/scrub/bmap_repair.c

+1 −5
Original line number Diff line number Diff line
@@ -923,7 +923,6 @@ xrep_bmap( 
	bool			allow_unwritten)

{

	struct xrep_bmap	*rb;

	char			*descr;

	xfs_extnum_t		max_bmbt_recs;

	bool			large_extcount;

	int			error = 0;
@@ -945,11 +944,8 @@ xrep_bmap( 
	/* Set up enough storage to handle the max records for this fork. */

	large_extcount = xfs_has_large_extent_counts(sc->mp);

	max_bmbt_recs = xfs_iext_max_nextents(large_extcount, whichfork);

	descr = xchk_xfile_ino_descr(sc, "%s fork mapping records",

			whichfork == XFS_DATA_FORK ? "data" : "attr");

	error = xfarray_create(descr, max_bmbt_recs,

	error = xfarray_create("fork mapping records", max_bmbt_recs,

			sizeof(struct xfs_bmbt_rec), &rb->bmap_records);

	kfree(descr);

	if (error)

		goto out_rb;

fs/xfs/scrub/common.h

+0 −25
Original line number Diff line number Diff line
@@ -246,31 +246,6 @@ static inline bool xchk_could_repair(const struct xfs_scrub *sc) 


int xchk_metadata_inode_forks(struct xfs_scrub *sc);



/*

 * Helper macros to allocate and format xfile description strings.

 * Callers must kfree the pointer returned.

 */

#define xchk_xfile_descr(sc, fmt, ...) \

	kasprintf(XCHK_GFP_FLAGS, "XFS (%s): " fmt, \

			(sc)->mp->m_super->s_id, ##__VA_ARGS__)

#define xchk_xfile_ag_descr(sc, fmt, ...) \

	kasprintf(XCHK_GFP_FLAGS, "XFS (%s): AG 0x%x " fmt, \

			(sc)->mp->m_super->s_id, \

			(sc)->sa.pag ? \

				pag_agno((sc)->sa.pag) : (sc)->sm->sm_agno, \

			##__VA_ARGS__)

#define xchk_xfile_ino_descr(sc, fmt, ...) \

	kasprintf(XCHK_GFP_FLAGS, "XFS (%s): inode 0x%llx " fmt, \

			(sc)->mp->m_super->s_id, \

			(sc)->ip ? (sc)->ip->i_ino : (sc)->sm->sm_ino, \

			##__VA_ARGS__)

#define xchk_xfile_rtgroup_descr(sc, fmt, ...) \

	kasprintf(XCHK_GFP_FLAGS, "XFS (%s): rtgroup 0x%x " fmt, \

			(sc)->mp->m_super->s_id, \

			(sc)->sa.pag ? \

				rtg_rgno((sc)->sr.rtg) : (sc)->sm->sm_agno, \

			##__VA_ARGS__)



/*

 * Setting up a hook to wait for intents to drain is costly -- we have to take

 * the CPU hotplug lock and force an i-cache flush on all CPUs once to set it