From: Ovidiu Panait <ovidiu.pan...@windriver.com>

Currently, some segfaults are reported when running ptest:
mdadm[12333]: segfault at 0 ip 00007fe855924060 sp 00007ffc4d6caf88 error 4 in 
libc.so.6[7f)
Code: d2 0f 84 b7 0f 00 00 48 83 fa 01 0f 84 b9 0f 00 00 49 89 d3 89 f1 89 f8 
48 83 e1 3f 4f

Backport the following upstream commits to fix them:
679bd9508a30 ("DDF: Cleanup validate_geometry_ddf_container()")
2b93288a5650 ("DDF: Fix NULL pointer dereference in validate_geometry_ddf()")
548e9b916f86 ("mdadm/Grow: Fix use after close bug by closing after fork")
9ae62977b51d ("monitor: Avoid segfault when calling NULL get_bad_blocks")

The fixes are part of the "Bug fixes and testing improvments" patchset [1].

[1] https://www.spinics.net/lists/raid/msg70621.html

Signed-off-by: Ovidiu Panait <ovidiu.pan...@windriver.com>
---
 ...anup-validate_geometry_ddf_container.patch | 148 ++++++++++++++++++
 ...nter-dereference-in-validate_geometr.patch |  56 +++++++
 ...se-after-close-bug-by-closing-after-.patch |  91 +++++++++++
 ...gfault-when-calling-NULL-get_bad_blo.patch |  42 +++++
 meta/recipes-extended/mdadm/mdadm_4.2.bb      |   4 +
 5 files changed, 341 insertions(+)
 create mode 100644 
meta/recipes-extended/mdadm/files/0001-DDF-Cleanup-validate_geometry_ddf_container.patch
 create mode 100644 
meta/recipes-extended/mdadm/files/0002-DDF-Fix-NULL-pointer-dereference-in-validate_geometr.patch
 create mode 100644 
meta/recipes-extended/mdadm/files/0003-mdadm-Grow-Fix-use-after-close-bug-by-closing-after-.patch
 create mode 100644 
meta/recipes-extended/mdadm/files/0004-monitor-Avoid-segfault-when-calling-NULL-get_bad_blo.patch

diff --git 
a/meta/recipes-extended/mdadm/files/0001-DDF-Cleanup-validate_geometry_ddf_container.patch
 
b/meta/recipes-extended/mdadm/files/0001-DDF-Cleanup-validate_geometry_ddf_container.patch
new file mode 100644
index 0000000000..cea435f83b
--- /dev/null
+++ 
b/meta/recipes-extended/mdadm/files/0001-DDF-Cleanup-validate_geometry_ddf_container.patch
@@ -0,0 +1,148 @@
+From ca458f4dcc4de9403298f67543466ce4bbc8f8ae Mon Sep 17 00:00:00 2001
+From: Logan Gunthorpe <log...@deltatee.com>
+Date: Wed, 22 Jun 2022 14:25:07 -0600
+Subject: [PATCH 1/4] DDF: Cleanup validate_geometry_ddf_container()
+
+Move the function up so that the function declaration is not necessary
+and remove the unused arguments to the function.
+
+No functional changes are intended but will help with a bug fix in the
+next patch.
+
+Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
+Acked-by: Mariusz Tkaczyk <mariusz.tkac...@linux.intel.com>
+Signed-off-by: Jes Sorensen <j...@trained-monkey.org>
+
+Upstream-Status: Backport
+
+Reference to upstream patch:
+https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=679bd9508a30
+
+Signed-off-by: Ovidiu Panait <ovidiu.pan...@windriver.com>
+---
+ super-ddf.c | 88 ++++++++++++++++++++++++-----------------------------
+ 1 file changed, 39 insertions(+), 49 deletions(-)
+
+diff --git a/super-ddf.c b/super-ddf.c
+index 3f304cd..65cf727 100644
+--- a/super-ddf.c
++++ b/super-ddf.c
+@@ -503,13 +503,6 @@ struct ddf_super {
+ static int load_super_ddf_all(struct supertype *st, int fd,
+                             void **sbp, char *devname);
+ static int get_svd_state(const struct ddf_super *, const struct vcl *);
+-static int
+-validate_geometry_ddf_container(struct supertype *st,
+-                              int level, int layout, int raiddisks,
+-                              int chunk, unsigned long long size,
+-                              unsigned long long data_offset,
+-                              char *dev, unsigned long long *freesize,
+-                              int verbose);
+ 
+ static int validate_geometry_ddf_bvd(struct supertype *st,
+                                    int level, int layout, int raiddisks,
+@@ -3322,6 +3315,42 @@ static int reserve_space(struct supertype *st, int 
raiddisks,
+       return 1;
+ }
+ 
++static int
++validate_geometry_ddf_container(struct supertype *st,
++                              int level, int raiddisks,
++                              unsigned long long data_offset,
++                              char *dev, unsigned long long *freesize,
++                              int verbose)
++{
++      int fd;
++      unsigned long long ldsize;
++
++      if (level != LEVEL_CONTAINER)
++              return 0;
++      if (!dev)
++              return 1;
++
++      fd = dev_open(dev, O_RDONLY|O_EXCL);
++      if (fd < 0) {
++              if (verbose)
++                      pr_err("ddf: Cannot open %s: %s\n",
++                             dev, strerror(errno));
++              return 0;
++      }
++      if (!get_dev_size(fd, dev, &ldsize)) {
++              close(fd);
++              return 0;
++      }
++      close(fd);
++      if (freesize) {
++              *freesize = avail_size_ddf(st, ldsize >> 9, INVALID_SECTORS);
++              if (*freesize == 0)
++                      return 0;
++      }
++
++      return 1;
++}
++
+ static int validate_geometry_ddf(struct supertype *st,
+                                int level, int layout, int raiddisks,
+                                int *chunk, unsigned long long size,
+@@ -3347,11 +3376,9 @@ static int validate_geometry_ddf(struct supertype *st,
+               level = LEVEL_CONTAINER;
+       if (level == LEVEL_CONTAINER) {
+               /* Must be a fresh device to add to a container */
+-              return validate_geometry_ddf_container(st, level, layout,
+-                                                     raiddisks, *chunk,
+-                                                     size, data_offset, dev,
+-                                                     freesize,
+-                                                     verbose);
++              return validate_geometry_ddf_container(st, level, raiddisks,
++                                                     data_offset, dev,
++                                                     freesize, verbose);
+       }
+ 
+       if (!dev) {
+@@ -3449,43 +3476,6 @@ static int validate_geometry_ddf(struct supertype *st,
+       return 1;
+ }
+ 
+-static int
+-validate_geometry_ddf_container(struct supertype *st,
+-                              int level, int layout, int raiddisks,
+-                              int chunk, unsigned long long size,
+-                              unsigned long long data_offset,
+-                              char *dev, unsigned long long *freesize,
+-                              int verbose)
+-{
+-      int fd;
+-      unsigned long long ldsize;
+-
+-      if (level != LEVEL_CONTAINER)
+-              return 0;
+-      if (!dev)
+-              return 1;
+-
+-      fd = dev_open(dev, O_RDONLY|O_EXCL);
+-      if (fd < 0) {
+-              if (verbose)
+-                      pr_err("ddf: Cannot open %s: %s\n",
+-                             dev, strerror(errno));
+-              return 0;
+-      }
+-      if (!get_dev_size(fd, dev, &ldsize)) {
+-              close(fd);
+-              return 0;
+-      }
+-      close(fd);
+-      if (freesize) {
+-              *freesize = avail_size_ddf(st, ldsize >> 9, INVALID_SECTORS);
+-              if (*freesize == 0)
+-                      return 0;
+-      }
+-
+-      return 1;
+-}
+-
+ static int validate_geometry_ddf_bvd(struct supertype *st,
+                                    int level, int layout, int raiddisks,
+                                    int *chunk, unsigned long long size,
+-- 
+2.39.1
+
diff --git 
a/meta/recipes-extended/mdadm/files/0002-DDF-Fix-NULL-pointer-dereference-in-validate_geometr.patch
 
b/meta/recipes-extended/mdadm/files/0002-DDF-Fix-NULL-pointer-dereference-in-validate_geometr.patch
new file mode 100644
index 0000000000..fafe88b49c
--- /dev/null
+++ 
b/meta/recipes-extended/mdadm/files/0002-DDF-Fix-NULL-pointer-dereference-in-validate_geometr.patch
@@ -0,0 +1,56 @@
+From 14f110f0286d38e29ef5e51d7f72e049c2f18323 Mon Sep 17 00:00:00 2001
+From: Logan Gunthorpe <log...@deltatee.com>
+Date: Wed, 22 Jun 2022 14:25:08 -0600
+Subject: [PATCH 2/4] DDF: Fix NULL pointer dereference in
+ validate_geometry_ddf()
+
+A relatively recent patch added a call to validate_geometry() in
+Manage_add() that has level=LEVEL_CONTAINER and chunk=NULL.
+
+This causes some ddf tests to segfault which aborts the test suite.
+
+To fix this, avoid dereferencing chunk when the level is
+LEVEL_CONTAINER or LEVEL_NONE.
+
+Fixes: 1f5d54a06df0 ("Manage: Call validate_geometry when adding drive to 
external container")
+Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
+Acked-by: Mariusz Tkaczyk <mariusz.tkac...@linux.intel.com>
+Signed-off-by: Jes Sorensen <j...@trained-monkey.org>
+
+Upstream-Status: Backport
+
+Reference to upstream patch:
+https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=2b93288a5650
+
+Signed-off-by: Ovidiu Panait <ovidiu.pan...@windriver.com>
+---
+ super-ddf.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/super-ddf.c b/super-ddf.c
+index 65cf727..3ef1293 100644
+--- a/super-ddf.c
++++ b/super-ddf.c
+@@ -3369,9 +3369,6 @@ static int validate_geometry_ddf(struct supertype *st,
+        * If given BVDs, we make an SVD, changing all the GUIDs in the process.
+        */
+ 
+-      if (*chunk == UnSet)
+-              *chunk = DEFAULT_CHUNK;
+-
+       if (level == LEVEL_NONE)
+               level = LEVEL_CONTAINER;
+       if (level == LEVEL_CONTAINER) {
+@@ -3381,6 +3378,9 @@ static int validate_geometry_ddf(struct supertype *st,
+                                                      freesize, verbose);
+       }
+ 
++      if (*chunk == UnSet)
++              *chunk = DEFAULT_CHUNK;
++
+       if (!dev) {
+               mdu_array_info_t array = {
+                       .level = level,
+-- 
+2.39.1
+
diff --git 
a/meta/recipes-extended/mdadm/files/0003-mdadm-Grow-Fix-use-after-close-bug-by-closing-after-.patch
 
b/meta/recipes-extended/mdadm/files/0003-mdadm-Grow-Fix-use-after-close-bug-by-closing-after-.patch
new file mode 100644
index 0000000000..a954ab027a
--- /dev/null
+++ 
b/meta/recipes-extended/mdadm/files/0003-mdadm-Grow-Fix-use-after-close-bug-by-closing-after-.patch
@@ -0,0 +1,91 @@
+From bd064da1469a6a07331b076a0294a8c6c3c38526 Mon Sep 17 00:00:00 2001
+From: Logan Gunthorpe <log...@deltatee.com>
+Date: Wed, 22 Jun 2022 14:25:09 -0600
+Subject: [PATCH 3/4] mdadm/Grow: Fix use after close bug by closing after fork
+
+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 <ale...@synology.com>
+Cc: BingJing Chang <bingji...@synology.com>
+Cc: Danny Shih <dannys...@synology.com>
+Cc: ChangSyun Peng <allenp...@synology.com>
+Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
+Acked-by: Mariusz Tkaczyk <mariusz.tkac...@linux.intel.com>
+Signed-off-by: Jes Sorensen <j...@trained-monkey.org>
+
+Upstream-Status: Backport
+
+Reference to upstream patch:
+https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=548e9b916f86
+
+Signed-off-by: Ovidiu Panait <ovidiu.pan...@windriver.com>
+---
+ Grow.c | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/Grow.c b/Grow.c
+index 9c6fc95..a8e4e83 100644
+--- a/Grow.c
++++ b/Grow.c
+@@ -3501,7 +3501,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.
+@@ -3522,6 +3521,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.39.1
+
diff --git 
a/meta/recipes-extended/mdadm/files/0004-monitor-Avoid-segfault-when-calling-NULL-get_bad_blo.patch
 
b/meta/recipes-extended/mdadm/files/0004-monitor-Avoid-segfault-when-calling-NULL-get_bad_blo.patch
new file mode 100644
index 0000000000..72cb40f782
--- /dev/null
+++ 
b/meta/recipes-extended/mdadm/files/0004-monitor-Avoid-segfault-when-calling-NULL-get_bad_blo.patch
@@ -0,0 +1,42 @@
+From 2296a4a441b4b8546e2eb32403930f1bb8f3ee4a Mon Sep 17 00:00:00 2001
+From: Logan Gunthorpe <log...@deltatee.com>
+Date: Wed, 22 Jun 2022 14:25:10 -0600
+Subject: [PATCH 4/4] monitor: Avoid segfault when calling NULL get_bad_blocks
+
+Not all struct superswitch implement a get_bad_blocks() function,
+yet mdmon seems to call it without checking for NULL and thus
+occasionally segfaults in the test 10ddf-geometry.
+
+Fix this by checking for NULL before calling it.
+
+Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
+Acked-by: Mariusz Tkaczyk <mariusz.tkac...@linux.intel.com>
+Signed-off-by: Jes Sorensen <j...@trained-monkey.org>
+
+Upstream-Status: Backport
+
+Reference to upstream patch:
+https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=9ae62977b51d
+
+Signed-off-by: Ovidiu Panait <ovidiu.pan...@windriver.com>
+---
+ monitor.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/monitor.c b/monitor.c
+index afc3e50..8e43c0d 100644
+--- a/monitor.c
++++ b/monitor.c
+@@ -312,6 +312,9 @@ static int check_for_cleared_bb(struct active_array *a, 
struct mdinfo *mdi)
+       struct md_bb *bb;
+       int i;
+ 
++      if (!ss->get_bad_blocks)
++              return -1;
++
+       /*
+        * Get a list of bad blocks for an array, then read list of
+        * acknowledged bad blocks from kernel and compare it against metadata
+-- 
+2.39.1
+
diff --git a/meta/recipes-extended/mdadm/mdadm_4.2.bb 
b/meta/recipes-extended/mdadm/mdadm_4.2.bb
index a5844b6726..033766dc89 100644
--- a/meta/recipes-extended/mdadm/mdadm_4.2.bb
+++ b/meta/recipes-extended/mdadm/mdadm_4.2.bb
@@ -32,6 +32,10 @@ SRC_URI = 
"${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.xz \
            file://0001-tests-fix-raid0-tests-for-0.90-metadata.patch \
            
file://0001-tests-00readonly-Run-udevadm-settle-before-setting-r.patch \
            
file://0001-tests-04update-metadata-avoid-passing-chunk-size-to.patch \
+           file://0001-DDF-Cleanup-validate_geometry_ddf_container.patch \
+           
file://0002-DDF-Fix-NULL-pointer-dereference-in-validate_geometr.patch \
+           
file://0003-mdadm-Grow-Fix-use-after-close-bug-by-closing-after-.patch \
+           
file://0004-monitor-Avoid-segfault-when-calling-NULL-get_bad_blo.patch \
            "
 
 SRC_URI[sha256sum] = 
"461c215670864bb74a4d1a3620684aa2b2f8296dffa06743f26dda5557acf01d"
-- 
2.39.1

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#183633): 
https://lists.openembedded.org/g/openembedded-core/message/183633
Mute This Topic: https://lists.openembedded.org/mt/99851484/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to