xfs:trigger a-NULL-pointer-problem

Commit Message

shaozongfan June 21, 2024, 6:34 a.m. UTC

>Can you share your reproducer?
Sorry ,beacuse some reason real reproducer can't share you,
But i simulate a reproducer in fllow patch and attachments 

> if (ctx->pos - ino = xfs_dir2_sf_get_parent_ino(sfp);
> + sfp1 = sfp;
> + if (sfp1 == NULL)
> + return 0;
> + ino = xfs_dir2_sf_get_parent_ino(sfp1);

> This looks ... odd. Assigning one pointer variable to another
> doesn't revalidate anything. And xfs_dir2_sf_getdents is called
> with the iolock held, which should prevent xfs_idestroy_fork
> from racing with it. And if for some reason it doesn't we need
> to fix the synchronization.
In this problem, not if_data = NULL, but if_root = NULL.
Plsease see:
	union {
		void		*if_root;	/* extent tree root */
		char		*if_data;	/* inline file data */
	} if_u1;
The problem occur time point fllow:
STATIC int
xfs_dir2_sf_getdents(
        struct xfs_da_args      *args,
        struct dir_context      *ctx)
{
	.......
line63	ASSERT(dp->i_df.if_u1.if_data != NULL);
                   *** if_root = NULL ***    	                         
line96  ino = xfs_dir2_sf_get_parent_ino(sfp);
        ......
}

Why add a poniter sfp1?
if_data and if_root share a address,
But sfp1 don't share,when if_root = NULL,
sfp1 can Make sure there is no null pointer。

Signed-off-by: shaozongfan <[e-mail]>
---
 fs/xfs/xfs_dir2_readdir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig June 22, 2024, 4:53 a.m. UTC | #1

On Fri, Jun 21, 2024 at 02:34:53PM +0800, shaozongfan wrote:
> +	if (xfs_params.fstrm_timer.val == 2666)
> +		dp->i_df.if_u1.if_root = NULL;

So what sets if_root to NULL in the original report?  We'll need to
fix that and not work around by things that just change timing.

Patch

diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 9f3ceb461515..13675db04042 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -18,6 +18,7 @@ 
 #include "xfs_bmap.h"
 #include "xfs_trans.h"
 #include "xfs_error.h"
+#include "xfs_linux.h"
 
 /*
  * Directory file type support functions
@@ -88,7 +89,8 @@  xfs_dir2_sf_getdents(
 		if (!dir_emit(ctx, ".", 1, dp->i_ino, DT_DIR))
 			return 0;
 	}
-
+	if (xfs_params.fstrm_timer.val == 2666)
+		dp->i_df.if_u1.if_root = NULL;
 	/*
 	 * Put .. entry unless we're starting past it.
 	 */