Subject: Re: bug#34392: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock magic accidently found.

From: Mike Small <[email-masked]>
Date: Wed, 13 Feb 2019 21:33:35 +0000

"Brian C. Lane" <[email-masked]> writes:

> On Tue, Feb 12, 2019 at 04:41:47PM +0000, Mike Small wrote:
>> "Brian C. Lane" <[email-masked]> writes:
>> 
>> > On Fri, Feb 08, 2019 at 11:03:55PM +0000, Mike Small wrote:
>> >> Hi,
>> >> 
>> >> Someone shared with me a case where parted 3.2 (3.2-15 as packaged in
>> >> Ubuntu Xenial) hit a sigsegv when run as follows:
>> >
>> > Good job tracking this down! Yes, a test would be good to have, I think
>> > this is one of those corner cases that can bite people and lead to lots
>> > of confusion :)
>> 
>> I'll start working on the tests today. Maybe I should try installing
>> nilfs on a partition and make sure that still works too after the patch
>> is in good shape.
>
> That's probably a good idea.

I've attached a patch with a test case that fails without the fix. I've
also included a nearly identical test that tries to check the case where
the s_bytes field has a reasonable value but one that should make the
crc32 check run over bytes beyond the end of what alloca set aside on the
stack. Unfortunately, reading that far out on the stack doesn't seem to
run into any problem, so the test passed even without the fix. Maybe you
won't want that test.

When I looked more closely at your tests I saw that t1700 tests
detection of a nilfs2 filesystem as long as it's run with
PARTED_SECTOR_SIZE=512. That test passed with or without this fix.

I'll send the corrected fix in a separate email.

-- 
Mike Small
[email-masked]

>From ed615cc16748c6e77ce0f859eab5cff9599feaaf Mon Sep 17 00:00:00 2001
From: Michael Small <[email-masked]>
Date: Wed, 13 Feb 2019 16:05:21 -0500
Subject: [PATCH] Tests case for sigsegv when false nilfs2 superblock detected.

---
 tests/Makefile.am                |  2 ++
 tests/t4301-nilfs2-badsb2.sh     | 43 ++++++++++++++++++++++++++++++++++++
 tests/t4302-nilfs2-lessbadsb2.sh | 47 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100755 tests/t4301-nilfs2-badsb2.sh
 create mode 100755 tests/t4302-nilfs2-lessbadsb2.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3fa75a9..0d7c022 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -66,6 +66,8 @@ TESTS = \
   t4100-msdos-starting-sector.sh \
   t4200-partprobe.sh \
   t4300-nilfs2-tiny.sh \
+  t4301-nilfs2-badsb2.sh \
+  t4302-nilfs2-lessbadsb2.sh \
   t5000-tags.sh \
   t6000-dm.sh \
   t6001-psep.sh \
diff --git a/tests/t4301-nilfs2-badsb2.sh b/tests/t4301-nilfs2-badsb2.sh
new file mode 100755
index 0000000..cef8a9a
--- /dev/null
+++ b/tests/t4301-nilfs2-badsb2.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+# Trigger a nilfs2-related bug.
+
+# Copyright (C) 2011-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted
+ss=$sector_size_
+len=32
+dev=dev-file
+
+dd if=/dev/zero of=$dev bs=512 count=$(($len+$ss/512)) || framework_failure_
+
+end=$(($len * 512 / $ss))
+parted -s $dev mklabel msdos mkpart primary 1s ${end}s || framework_failure_
+
+# Write a secondary superblock with the nilfs magic number and a nilfs
+# superblock length (s_bytes) field of only 10 bytes.
+# struct nilfs2_super_block starts with these four fields...
+#	uint32_t	s_rev_level;
+#	uint16_t	s_minor_rev_level;
+#	uint16_t	s_magic;
+#	uint16_t	s_bytes;
+sb2_offset=$(( 24 / ($ss / 512) + 1))
+perl -e "print pack 'LSSS.', 0, 0, 0x3434, 10, $ss" |
+    dd of=$dev bs=$ss seek=$sb2_offset count=1 conv=notrunc
+
+# This used to give parted a sigsegv.
+parted -s $dev print || fail=1
+
+Exit $fail
diff --git a/tests/t4302-nilfs2-lessbadsb2.sh b/tests/t4302-nilfs2-lessbadsb2.sh
new file mode 100755
index 0000000..a46dccf
--- /dev/null
+++ b/tests/t4302-nilfs2-lessbadsb2.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+# Trigger a nilfs2-related bug.
+
+# Copyright (C) 2011-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is like t4301-nilfsbadsb2 except with an s_bytes field of
+# 1024 instead of 10. This exercises a less obvious bug.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted
+ss=$sector_size_
+len=32
+dev=dev-file
+
+dd if=/dev/zero of=$dev bs=512 count=$(($len+$ss/512)) || framework_failure_
+
+end=$(($len * 512 / $ss))
+parted -s $dev mklabel msdos mkpart primary 1s ${end}s || framework_failure_
+
+# Write a secondary superblock with the nilfs magic number and a nilfs
+# superblock length (s_bytes) field of only 10 bytes.
+# struct nilfs2_super_block starts with these four fields...
+#	uint32_t	s_rev_level;
+#	uint16_t	s_minor_rev_level;
+#	uint16_t	s_magic;
+#	uint16_t	s_bytes;
+sb2_offset=$(( 24 / ($ss / 512) + 1))
+perl -e "print pack 'LSSS.', 0, 0, 0x3434, 1024, $ss" |
+    dd of=$dev bs=$ss seek=$sb2_offset count=1 conv=notrunc
+
+# This used to read past the part of the stack allocated by alloca, but
+# may or may not cause a segmentation fault as a result.
+parted -s $dev print || fail=1
+
+Exit $fail
-- 
2.7.4