fs/xfs/xfs_dir2_readdir.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
when a process using getdents64() api to get a Folder inside
the file directory,meantime other process delete the file
directory.it would cause an error like this:
[ 100.640099] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000001
[ 100.641246] Mem abort info:
[ 100.641636] ESR = 0x96000007
[ 100.642057] Exception class = DABT (current EL), IL = 32 bits
[ 100.642815] SET = 0, FnV = 0
[ 100.643235] EA = 0, S1PTW = 0
[ 100.643664] Data abort info:
[ 100.644063] ISV = 0, ISS = 0x00000007
[ 100.644574] CM = 0, WnR = 0
[ 100.644984] user pgtable: 64k pages, 48-bit VAs, pgdp = 00000000135105a5
[ 100.645876] [0000000000000001] pgd=00000001ac6b0003, pud=00000001ac6b0003, pmd=00000001aad30003, pte=0000000000000000
[ 100.647204] Internal error: Oops: 96000007 [#1] SMP
[ 100.647843] Modules linked in: binfmt_misc fuse devlink xt_CHECKSUM ipt_MASQUERADE ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ip6table_nat nf_nat_ipv6 ip6table_mangle tun bridge ip6table_raw ip6table_security stp llc iptable_nat nf_nat_ipv4 nf_nat iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sch_fq_codel sunrpc vfat fat ip_tables sr_mod cdrom virtio_gpu virtio_console virtio_net virtio_scsi net_failover failover dm_mirror dm_region_hash dm_log gb
[ 100.654877] Process getdents (pid: 6455, stack limit = 0x00000000a8241109)
[ 100.655759] CPU: 0 PID: 6455 Comm: getdents Kdump: loaded Tainted: P OE 4.19.90-25.10.v2101.ky10.aarch64 #1
[ 100.657418] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[ 100.658484] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 100.659297] pc : xfs_dir2_sf_get_parent_ino+0x1c/0x30
[ 100.660093] lr : xfs_dir2_sf_getdents.isra.0+0xd8/0x248
[ 100.660922] sp : ffff80016eda3bd0
[ 100.661476] x29: ffff80016eda3bd0 x28: ffff800167c4d900
[ 100.662310] x27: 0000000000000000 x26: 0000000000000000
[ 100.663149] x25: 0000000000000000 x24: ffff800174ca5000
[ 100.663980] x23: 000000000000000a x22: 0000000000000000
[ 100.664822] x21: ffff800161337780 x20: ffff8001437b8000
[ 100.665647] x19: 0000000000000000 x18: 0000000000000000
[ 100.666482] x17: 0000000000000001 x16: 0000000000000000
[ 100.667328] x15: 0000000000000000 x14: 0000000000000000
[ 100.668159] x13: 0000000000000000 x12: 0000000000000000
[ 100.668985] x11: 0000000000000000 x10: 0000000000000000
[ 100.669816] x9 : 0000000000000000 x8 : 0000000000000000
[ 100.670643] x7 : 0000000000000000 x6 : ffff8001704a6414
[ 100.671517] x5 : 0000000000000004 x4 : 0000000004195704
[ 100.672355] x3 : 000000000000002e x2 : 0000000000000001
[ 100.673181] x1 : 0000000000000002 x0 : ffff0000084cda20
[ 100.674007] Call trace:
[ 100.674427] xfs_dir2_sf_get_parent_ino+0x1c/0x30
[ 100.675168] xfs_dir2_sf_getdents.isra.0+0xd8/0x248
[ 100.675943] xfs_readdir+0x184/0x1d0
[ 100.676522] xfs_file_readdir+0x40/0x50
[ 100.677149] iterate_dir+0x8c/0x1a8
[ 100.677717] ksys_getdents64+0xb4/0x348
[ 100.678335] __arm64_sys_getdents64+0x28/0x38
[ 100.679027] el0_svc_common+0x78/0x130
[ 100.679640] el0_svc_handler+0x38/0x78
[ 100.680249] el0_svc+0x8/0x1b0
[ 100.680756] Code: aa0003f3 aa1e03e0 d503201f 91000a61 (39400660)
[ 100.681720] SMP: stopping secondary CPUs
[ 100.684250] Starting crashdump kernel...
[ 100.684868] Bye!
Signed-off-by: shaozongfan <shaozongfan@kylinos.cn>
---
fs/xfs/xfs_dir2_readdir.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 9f3ceb461515..db6c910cad96 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -51,7 +51,7 @@ xfs_dir2_sf_getdents(
struct xfs_mount *mp = dp->i_mount;
xfs_dir2_dataptr_t off; /* current entry's offset */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
+ xfs_dir2_sf_hdr_t *sfp, *sfp1; /* shortform structure */
xfs_dir2_dataptr_t dot_offset;
xfs_dir2_dataptr_t dotdot_offset;
xfs_ino_t ino;
@@ -93,7 +93,10 @@ xfs_dir2_sf_getdents(
* Put .. entry unless we're starting past it.
*/
if (ctx->pos <= dotdot_offset) {
- ino = xfs_dir2_sf_get_parent_ino(sfp);
+ sfp1 = sfp;
+ if (sfp1 == NULL)
+ return 0;
+ ino = xfs_dir2_sf_get_parent_ino(sfp1);
ctx->pos = dotdot_offset & 0x7fffffff;
if (!dir_emit(ctx, "..", 2, ino, DT_DIR))
return 0;
--
2.25.1On Thu, Jun 20, 2024 at 03:43:13PM +0800, shaozongfan wrote:
> when a process using getdents64() api to get a Folder inside
> the file directory,meantime other process delete the file
> directory.it would cause an error like this:
Can you share your reproducer?
> if (ctx->pos <= dotdot_offset) {
> - 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.
>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 <shaozongfan@kylinos.cn>
---
fs/xfs/xfs_dir2_readdir.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
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.
*/
--
2.25.1On 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.
© 2016 - 2025 Red Hat, Inc.