From: Logan Gunthorpe <logang@deltatee.com>
To: linux-raid@vger.kernel.org, Jes Sorensen <jsorensen@fb.com>
Cc: Song Liu <song@kernel.org>, Christoph Hellwig <hch@infradead.org>,
Donald Buczek <buczek@molgen.mpg.de>,
Guoqing Jiang <guoqing.jiang@linux.dev>, Xiao Ni <xni@redhat.com>,
Himanshu Madhani <himanshu.madhani@oracle.com>,
Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>,
Coly Li <colyli@suse.de>, Bruce Dubbs <bruce.dubbs@gmail.com>,
Stephen Bates <sbates@raithlin.com>,
Martin Oliveira <Martin.Oliveira@eideticom.com>,
David Sloan <David.Sloan@eideticom.com>,
Logan Gunthorpe <logang@deltatee.com>,
Alex Wu <alexwu@synology.com>,
BingJing Chang <bingjingc@synology.com>,
Danny Shih <dannyshih@synology.com>,
ChangSyun Peng <allenpeng@synology.com>
Subject: [PATCH mdadm v2 04/14] mdadm/Grow: Fix use after close bug by closing after fork
Date: Wed, 22 Jun 2022 14:25:09 -0600 [thread overview]
Message-ID: <20220622202519.35905-5-logang@deltatee.com> (raw)
In-Reply-To: <20220622202519.35905-1-logang@deltatee.com>
The test 07reshape-grow fails most of the time. But it succeeds around
1 in 5 times. When it does succeed, it causes the tests to die because
mdadm has segfaulted.
The segfault was caused by mdadm attempting to repoen a file
descriptor that was already closed. The backtrace of the segfault
was:
#0 __strncmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:101
#1 0x000056146e31d44b in devnm2devid (devnm=0x0) at util.c:956
#2 0x000056146e31dab4 in open_dev_flags (devnm=0x0, flags=0)
at util.c:1072
#3 0x000056146e31db22 in open_dev (devnm=0x0) at util.c:1079
#4 0x000056146e3202e8 in reopen_mddev (mdfd=4) at util.c:2244
#5 0x000056146e329f36 in start_array (mdfd=4,
mddev=0x7ffc55342450 "/dev/md0", content=0x7ffc55342860,
st=0x56146fc78660, ident=0x7ffc55342f70, best=0x56146fc6f5d0,
bestcnt=10, chosen_drive=0, devices=0x56146fc706b0, okcnt=5,
sparecnt=0, rebuilding_cnt=0, journalcnt=0, c=0x7ffc55342e90,
clean=1, avail=0x56146fc78720 "\001\001\001\001\001",
start_partial_ok=0, err_ok=0, was_forced=0)
at Assemble.c:1206
#6 0x000056146e32c36e in Assemble (st=0x56146fc78660,
mddev=0x7ffc55342450 "/dev/md0", ident=0x7ffc55342f70,
devlist=0x56146fc6e2d0, c=0x7ffc55342e90)
at Assemble.c:1914
#7 0x000056146e312ac9 in main (argc=11, argv=0x7ffc55343238)
at mdadm.c:1510
The file descriptor was closed early in Grow_continue(). The noted commit
moved the close() call to close the fd above the fork which caused the
parent process to return with a closed fd.
This meant reshape_array() and Grow_continue() would return in the parent
with the fd forked. The fd would eventually be passed to reopen_mddev()
which returned an unhandled NULL from fd2devnm() which would then be
dereferenced in devnm2devid.
Fix this by moving the close() call below the fork. This appears to
fix the 07revert-grow test. While we're at it, switch to using
close_fd() to invalidate the file descriptor.
Fixes: 77b72fa82813 ("mdadm/Grow: prevent md's fd from being occupied during delayed time")
Cc: Alex Wu <alexwu@synology.com>
Cc: BingJing Chang <bingjingc@synology.com>
Cc: Danny Shih <dannyshih@synology.com>
Cc: ChangSyun Peng <allenpeng@synology.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
Grow.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Grow.c b/Grow.c
index f6efbc48dafd..0e2d7181bcab 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3514,7 +3514,6 @@ started:
return 0;
}
- close(fd);
/* Now we just need to kick off the reshape and watch, while
* handling backups of the data...
* This is all done by a forked background process.
@@ -3535,6 +3534,9 @@ started:
break;
}
+ /* Close unused file descriptor in the forked process */
+ close_fd(&fd);
+
/* If another array on the same devices is busy, the
* reshape will wait for them. This would mean that
* the first section that we suspend will stay suspended
--
2.30.2
next prev parent reply other threads:[~2022-06-22 20:25 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-22 20:25 [PATCH mdadm v2 00/14] Bug fixes and testing improvments Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 01/14] Makefile: Don't build static build with everything and everything-test Logan Gunthorpe
2022-06-28 7:00 ` Mariusz Tkaczyk
2022-06-22 20:25 ` [PATCH mdadm v2 02/14] DDF: Cleanup validate_geometry_ddf_container() Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 03/14] DDF: Fix NULL pointer dereference in validate_geometry_ddf() Logan Gunthorpe
2022-06-22 20:25 ` Logan Gunthorpe [this message]
2022-06-28 7:02 ` [PATCH mdadm v2 04/14] mdadm/Grow: Fix use after close bug by closing after fork Mariusz Tkaczyk
2022-06-22 20:25 ` [PATCH mdadm v2 05/14] monitor: Avoid segfault when calling NULL get_bad_blocks Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 06/14] mdadm: Fix mdadm -r remove option regression Logan Gunthorpe
2022-06-28 7:03 ` Mariusz Tkaczyk
2022-06-22 20:25 ` [PATCH mdadm v2 07/14] mdadm: Fix optional --write-behind parameter Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 08/14] tests/00raid0: add a test that validates raid0 with layout fails for 0.9 Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 09/14] tests: fix raid0 tests for 0.90 metadata Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 10/14] tests/04update-metadata: avoid passing chunk size to raid1 Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 11/14] tests/02lineargrow: clear the superblock at every iteration Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 12/14] mdadm/test: Add a mode to repeat specified tests Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 13/14] mdadm/test: Mark and ignore broken test failures Logan Gunthorpe
2022-06-22 20:25 ` [PATCH mdadm v2 14/14] tests: Add broken files for all broken tests Logan Gunthorpe
2022-07-22 17:00 ` [PATCH mdadm v2 00/14] Bug fixes and testing improvments Himanshu Madhani
2022-07-23 6:21 ` Coly Li
2022-08-08 20:22 ` Himanshu Madhani
2022-08-07 20:35 ` Jes Sorensen
2022-08-08 15:46 ` Logan Gunthorpe
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=20220622202519.35905-5-logang@deltatee.com \
--to=logang@deltatee.com \
--cc=David.Sloan@eideticom.com \
--cc=Martin.Oliveira@eideticom.com \
--cc=alexwu@synology.com \
--cc=allenpeng@synology.com \
--cc=bingjingc@synology.com \
--cc=bruce.dubbs@gmail.com \
--cc=buczek@molgen.mpg.de \
--cc=colyli@suse.de \
--cc=dannyshih@synology.com \
--cc=guoqing.jiang@linux.dev \
--cc=hch@infradead.org \
--cc=himanshu.madhani@oracle.com \
--cc=jsorensen@fb.com \
--cc=linux-raid@vger.kernel.org \
--cc=mariusz.tkaczyk@linux.intel.com \
--cc=sbates@raithlin.com \
--cc=song@kernel.org \
--cc=xni@redhat.com \
/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 NNTP newsgroup(s).