GNU bug report logs - #34392
[PATCH] Avoid sigsegv in case 2nd nilfs2 superblock magic accidently found.

Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.

Package: parted; Reported by: Mike Small <smallm@HIDDEN>; Keywords: patch; dated Fri, 8 Feb 2019 23:12:01 UTC; Maintainer for parted is bug-parted@HIDDEN.

Message received at 34392 <at> debbugs.gnu.org:


Received: (at 34392) by debbugs.gnu.org; 12 Feb 2019 17:56:13 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Feb 12 12:56:13 2019
Received: from localhost ([127.0.0.1]:45146 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1gtcI1-0001pi-56
	for submit <at> debbugs.gnu.org; Tue, 12 Feb 2019 12:56:13 -0500
Received: from mx1.redhat.com ([209.132.183.28]:43228)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <bcl@HIDDEN>) id 1gtcHz-0001pV-Nw
 for 34392 <at> debbugs.gnu.org; Tue, 12 Feb 2019 12:56:12 -0500
Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com
 [10.5.11.11])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id 3536FAC5E8;
 Tue, 12 Feb 2019 17:56:06 +0000 (UTC)
Received: from lister.brianlane.com (ovpn-112-38.phx2.redhat.com [10.3.112.38])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id 75C2D6019F;
 Tue, 12 Feb 2019 17:56:05 +0000 (UTC)
Date: Tue, 12 Feb 2019 09:56:02 -0800
From: "Brian C. Lane" <bcl@HIDDEN>
To: Mike Small <smallm@HIDDEN>
Subject: Re: bug#34392: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock
 magic accidently found.
Message-ID: <20190212175602.GI4594@HIDDEN>
References: <chximxtnaqs.fsf@HIDDEN>
 <20190211195931.GG4594@HIDDEN>
 <chx1s4d7yd0.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <chx1s4d7yd0.fsf@HIDDEN>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.5.110.28]); Tue, 12 Feb 2019 17:56:06 +0000 (UTC)
X-Spam-Score: -5.0 (-----)
X-Debbugs-Envelope-To: 34392
Cc: 34392 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -6.0 (------)

On Tue, Feb 12, 2019 at 04:41:47PM +0000, Mike Small wrote:
> "Brian C. Lane" <bcl@HIDDEN> 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.

> 
> >
> >>  	crc = __efi_crc32(sb, sumoff, PED_LE32_TO_CPU(sb->s_crc_seed));
> >> @@ -113,11 +113,13 @@ nilfs2_probe (PedGeometry* geom)
> >>  	const int sectors = (4096 + geom->dev->sector_size - 1) /
> >>  			     geom->dev->sector_size;
> >>  	char *buf = alloca (sectors * geom->dev->sector_size);
> >> -	void *buff2 = alloca (geom->dev->sector_size);
> >> +	const int sectors2 = sizeof(struct nilfs2_super_block) / geom->dev->sector_size +
> >> +                (sizeof(struct nilfs2_super_block) % geom->dev->sector_size == 0) ? 0 : 1;
> >
> > This calculation is correct, but I find it hard to read. If you use the
> > same technique as it does for sectors it would be easier to understand
> > in the future, and I don't think the superblock size is going to change.
> 
> Probably I should have spent more time trying to understand the way
> sectors was calculated or asked about it before submitting the patch. It
> confused me, since in my case, where geom->dev->sector_size was 512,
> that calculation gave a size that meant eight 512 byte sectors were read
> instead of two (sizeof nilfs2_super_block = 1024):
> 
> (4096 + 512 - 1) / 512 = 8.
> 
> And that's what it did, except all at once, based on the strace...
> 
> lseek(3, 11813257216, SEEK_SET)         = 11813257216
> read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
> 
> And then there was the 1024 offset introduced when assigning to the
> primary superblock, sb, which I didn't understand the purpose of...
> 
> 	if (ped_geometry_read(geom, buf, 0, sectors))
> 		sb = (struct nilfs2_super_block *)(buf+1024);
> 
> 
> I wasn't sure if reading the extra six sectors for the 2nd superblock
> would be okay, e.g. if the superblock was really close to the end of a
> disk. And in general there are these things about reading the first
> superblock which I don't understand, so I'm unclear if the two lengths
> should be computed the same way. If so should we look for the 2nd
> superblock 1024 bytes into the 4096 bytes read like we do for the 1st
> superblock?

I can't seem to find a decent reference for NILFS other than this code
and the linux kernel code so I'm not sure why it reads so much for the
first one. I think you've got the logic right, I just think it would be
easier to read as:

sectors2 = (1024 + geom->dev->sector_size - 1) / geom->dev->sector_size;

When reading the 2nd superblock it looks like it starts on a sector
boundary so that's why it doesn't need the 4096 offset.

-- 
Brian C. Lane (PST8PDT)




Information forwarded to bug-parted@HIDDEN:
bug#34392; Package parted. Full text available.

Message received at 34392 <at> debbugs.gnu.org:


Received: (at 34392) by debbugs.gnu.org; 12 Feb 2019 16:42:04 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Feb 12 11:42:04 2019
Received: from localhost ([127.0.0.1]:45085 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1gtb8G-0006Lv-Ag
	for submit <at> debbugs.gnu.org; Tue, 12 Feb 2019 11:42:04 -0500
Received: from ol.sdf.org ([205.166.94.20]:65419 helo=mx.sdf.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <smallm@HIDDEN>) id 1gtb8D-0006LQ-2T
 for 34392 <at> debbugs.gnu.org; Tue, 12 Feb 2019 11:42:02 -0500
Received: from sdf.org (IDENT:smallm@HIDDEN [205.166.94.5])
 by mx.sdf.org (8.15.2/8.14.5) with ESMTPS id x1CGfnrt006600
 (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256 bits) verified NO); 
 Tue, 12 Feb 2019 16:41:50 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=sdf.org; s=default;
 t=1549989718; bh=6d0seM3wobsqwG1OLaQO1srFburNq8WQ++naSdUR1nQ=;
 h=From:To:Cc:Subject:References:Date:In-Reply-To;
 b=IepatmMWwjWX4j2vUtIRkfo8Ew9zFIq3nG/9/NlA9zn070bFlJKJDgFXHhejIw86U
 BpwocEv5T8rNHGtdqSyhXiCkTpKbYIT7v/PY5DrSscREWdxfHpTqRtsVftVH3W8oEU
 mYUSZrYsGqTa1RWTcIF/rTr4un5vcok4krwUfwEI=
Received: (from smallm@localhost)
 by sdf.org (8.15.2/8.12.8/Submit) id x1CGfmhf024053;
 Tue, 12 Feb 2019 16:41:48 GMT
From: Mike Small <smallm@HIDDEN>
To: "Brian C. Lane" <bcl@HIDDEN>
Subject: Re: bug#34392: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock
 magic accidently found.
References: <chximxtnaqs.fsf@HIDDEN>
 <20190211195931.GG4594@HIDDEN>
Date: Tue, 12 Feb 2019 16:41:47 +0000
In-Reply-To: <20190211195931.GG4594@HIDDEN> (Brian C. Lane's
 message of "Mon, 11 Feb 2019 11:59:31 -0800")
Message-ID: <chx1s4d7yd0.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (berkeley-unix)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 34392
Cc: 34392 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

"Brian C. Lane" <bcl@HIDDEN> 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.

>
>>  	crc = __efi_crc32(sb, sumoff, PED_LE32_TO_CPU(sb->s_crc_seed));
>> @@ -113,11 +113,13 @@ nilfs2_probe (PedGeometry* geom)
>>  	const int sectors = (4096 + geom->dev->sector_size - 1) /
>>  			     geom->dev->sector_size;
>>  	char *buf = alloca (sectors * geom->dev->sector_size);
>> -	void *buff2 = alloca (geom->dev->sector_size);
>> +	const int sectors2 = sizeof(struct nilfs2_super_block) / geom->dev->sector_size +
>> +                (sizeof(struct nilfs2_super_block) % geom->dev->sector_size == 0) ? 0 : 1;
>
> This calculation is correct, but I find it hard to read. If you use the
> same technique as it does for sectors it would be easier to understand
> in the future, and I don't think the superblock size is going to change.

Probably I should have spent more time trying to understand the way
sectors was calculated or asked about it before submitting the patch. It
confused me, since in my case, where geom->dev->sector_size was 512,
that calculation gave a size that meant eight 512 byte sectors were read
instead of two (sizeof nilfs2_super_block = 1024):

(4096 + 512 - 1) / 512 = 8.

And that's what it did, except all at once, based on the strace...

lseek(3, 11813257216, SEEK_SET)         = 11813257216
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096

And then there was the 1024 offset introduced when assigning to the
primary superblock, sb, which I didn't understand the purpose of...

	if (ped_geometry_read(geom, buf, 0, sectors))
		sb = (struct nilfs2_super_block *)(buf+1024);


I wasn't sure if reading the extra six sectors for the 2nd superblock
would be okay, e.g. if the superblock was really close to the end of a
disk. And in general there are these things about reading the first
superblock which I don't understand, so I'm unclear if the two lengths
should be computed the same way. If so should we look for the 2nd
superblock 1024 bytes into the 4096 bytes read like we do for the 1st
superblock?


-- 
Mike Small
smallm@HIDDEN




Information forwarded to bug-parted@HIDDEN:
bug#34392; Package parted. Full text available.

Message received at 34392 <at> debbugs.gnu.org:


Received: (at 34392) by debbugs.gnu.org; 11 Feb 2019 19:59:43 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon Feb 11 14:59:43 2019
Received: from localhost ([127.0.0.1]:44128 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1gtHjy-0003h0-NL
	for submit <at> debbugs.gnu.org; Mon, 11 Feb 2019 14:59:42 -0500
Received: from mx1.redhat.com ([209.132.183.28]:37566)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <bcl@HIDDEN>) id 1gtHjv-0003gm-UC
 for 34392 <at> debbugs.gnu.org; Mon, 11 Feb 2019 14:59:41 -0500
Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com
 [10.5.11.22])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id 396A6C0740E4;
 Mon, 11 Feb 2019 19:59:34 +0000 (UTC)
Received: from lister.brianlane.com (ovpn-112-38.phx2.redhat.com [10.3.112.38])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id A95B610027DA;
 Mon, 11 Feb 2019 19:59:33 +0000 (UTC)
Date: Mon, 11 Feb 2019 11:59:31 -0800
From: "Brian C. Lane" <bcl@HIDDEN>
To: Mike Small <smallm@HIDDEN>
Subject: Re: bug#34392: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock
 magic accidently found.
Message-ID: <20190211195931.GG4594@HIDDEN>
References: <chximxtnaqs.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <chximxtnaqs.fsf@HIDDEN>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.5.110.31]); Mon, 11 Feb 2019 19:59:34 +0000 (UTC)
X-Spam-Score: -5.0 (-----)
X-Debbugs-Envelope-To: 34392
Cc: 34392 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -6.0 (------)

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 :)

>  	crc = __efi_crc32(sb, sumoff, PED_LE32_TO_CPU(sb->s_crc_seed));
> @@ -113,11 +113,13 @@ nilfs2_probe (PedGeometry* geom)
>  	const int sectors = (4096 + geom->dev->sector_size - 1) /
>  			     geom->dev->sector_size;
>  	char *buf = alloca (sectors * geom->dev->sector_size);
> -	void *buff2 = alloca (geom->dev->sector_size);
> +	const int sectors2 = sizeof(struct nilfs2_super_block) / geom->dev->sector_size +
> +                (sizeof(struct nilfs2_super_block) % geom->dev->sector_size == 0) ? 0 : 1;

This calculation is correct, but I find it hard to read. If you use the
same technique as it does for sectors it would be easier to understand
in the future, and I don't think the superblock size is going to change.

-- 
Brian C. Lane (PST8PDT)




Information forwarded to bug-parted@HIDDEN:
bug#34392; Package parted. Full text available.

Message received at submit <at> debbugs.gnu.org:


Received: (at submit) by debbugs.gnu.org; 8 Feb 2019 23:11:54 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Fri Feb 08 18:11:53 2019
Received: from localhost ([127.0.0.1]:40289 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1gsFJJ-0003Dm-Fj
	for submit <at> debbugs.gnu.org; Fri, 08 Feb 2019 18:11:53 -0500
Received: from eggs.gnu.org ([209.51.188.92]:44883)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <smallm@HIDDEN>) id 1gsFBz-00030H-3G
 for submit <at> debbugs.gnu.org; Fri, 08 Feb 2019 18:04:20 -0500
Received: from lists.gnu.org ([209.51.188.17]:52262)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <smallm@HIDDEN>) id 1gsFBt-0006KF-VJ
 for submit <at> debbugs.gnu.org; Fri, 08 Feb 2019 18:04:14 -0500
Received: from eggs.gnu.org ([209.51.188.92]:59259)
 by lists.gnu.org with esmtp (Exim 4.71)
 (envelope-from <smallm@HIDDEN>) id 1gsFBs-0008Gn-AP
 for bug-parted@HIDDEN; Fri, 08 Feb 2019 18:04:13 -0500
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org
X-Spam-Level: 
X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,RCVD_IN_DNSWL_NONE,
 URIBL_BLOCKED autolearn=disabled version=3.3.2
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <smallm@HIDDEN>) id 1gsFBq-0006Ge-Tw
 for bug-parted@HIDDEN; Fri, 08 Feb 2019 18:04:12 -0500
Received: from mx.sdf.org ([205.166.94.20]:51291)
 by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <smallm@HIDDEN>) id 1gsFBq-0006DE-IA
 for bug-parted@HIDDEN; Fri, 08 Feb 2019 18:04:10 -0500
Received: from sdf.org (IDENT:smallm@HIDDEN [205.166.94.5])
 by mx.sdf.org (8.15.2/8.14.5) with ESMTPS id x18N3tUf012724
 (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256 bits) verified NO)
 for <bug-parted@HIDDEN>; Fri, 8 Feb 2019 23:03:56 GMT
Received: (from smallm@localhost)
 by sdf.org (8.15.2/8.12.8/Submit) id x18N3t7j020178;
 Fri, 8 Feb 2019 23:03:55 GMT
From: Mike Small <smallm@HIDDEN>
To: bug-parted@HIDDEN
Subject: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock magic accidently
 found.
Date: Fri, 08 Feb 2019 23:03:55 +0000
Message-ID: <chximxtnaqs.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
X-detected-operating-system: by eggs.gnu.org: Genre and OS details not
 recognized.
X-Received-From: 205.166.94.20
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: submit
X-Mailman-Approved-At: Fri, 08 Feb 2019 18:11:52 -0500
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

--=-=-=
Content-Type: text/plain

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:

parted -m -s /dev/sda print

When I looked into it, it appeared that they were extremely, extremely
unlucky. It's not the same nilfs problem Jim Meyering fixed back in
3.1. They just happened to have data that looked like the magic number
for a nilfs2 superblock in just the right place for parted to think
there might be a secondary nilfs superblock. So parted tried to do a
crc32 check on that sector (+ 512 more bytes beyond the end of it), but
with most of the struct being invalid in ways that led to reading beyond
the buffer allocated by alloca in nilfs2_probe().

The partition table looked like this (using sfdisk here since I don't
haven't put my fixed version of parted on the machine yet):

~# sfdisk -l /dev/sda
Disk /dev/sda: 233.8 GiB, 251059544064 bytes, 490350672 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x00000000

Device     Boot    Start       End   Sectors   Size Id Type
/dev/sda1           4096  10489855  10485760     5G 83 Linux
/dev/sda2       10489856  23072767  12582912     6G 83 Linux
/dev/sda3       23072768  60821503  37748736    18G 83 Linux
/dev/sda4       60821504 490350591 429529088 204.8G 83 Linux

The strace just before the sigsegv shows the seeks and reads, one near
the beginning and one near the end of sd3, that happen in
is_valid_nilfs_sb():

...
read(3, "C\16\322EC\213\234\224i(-f\365,\214\256\n\247\"x\350\0372\n0%]\242\5QJ\16"..., 512) = 512
lseek(3, 7168, SEEK_SET)                = 7168
read(3, "F\241\245\35\260\263\306\7\2\211U\16\326\275ph\225\370\273\222\272Q\332\274\346\323\365\251\370f?\5"..., 512) = 512
lseek(3, 7680, SEEK_SET)                = 7680
read(3, "\340\216\364*\365\347\25H\373\4|\33FQ\23\252\376tX:\345\227\342!\324(j;k-\227b"..., 512) = 512
lseek(3, 5370806272, SEEK_SET)          = 5370806272
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
lseek(3, 11813388288, SEEK_SET)         = 11813388288
read(3, " \200\0\0 \200\1\0 \200\2\0 \200\3\0 \200\4\0 \200\f\0 \200\r\0 \200\30\0"..., 512) = 512
lseek(3, 11813322752, SEEK_SET)         = 11813322752
read(3, "\20\200\0\0\20\200\1\0\20\200\2\0\20\200\3\0\20\200\4\0\20\200\f\0\20\200\r\0\20\200\30\0"..., 512) = 512
lseek(3, 11813257216, SEEK_SET)         = 11813257216
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 4096
lseek(3, 31140605952, SEEK_SET)         = 31140605952
read(3, "42 42 44\n\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x7ffcddd26000} ---
write(2, "\n\nYou found a bug in GNU Parted!"..., 828
...

The person sent me these sectors, so I was able to create a vm with the
same layout and dd in the unfortunate sector data at byte 31140605952
(sector 60821496).  With that I reproduced the bug with gdb and saw this
stack trace. Notice the value of len passed to __efi_crc32():

(gdb) set args -s /dev/vda print
(gdb) run
Starting program: /root/parted/parted/.libs/lt-parted -s /dev/vda print
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ba8bbe in __efi_crc32 (buf=0x7fffffffd3c4, len=18446744073709551606, 
    seed=2213123465) at efi_crc32.c:122
122     efi_crc32.c: No such file or directory.
(gdb) bt
#0  0x00007ffff7ba8bbe in __efi_crc32 (buf=0x7fffffffd3c4, len=18446744073709551606, 
    seed=2213123465) at efi_crc32.c:122
#1  0x00007ffff7b9f571 in is_valid_nilfs_sb (sb=0x7fffffffd3b0) at nilfs2/nilfs2.c:97
#2  0x00007ffff7b9f764 in nilfs2_probe (geom=0x61de38) at nilfs2/nilfs2.c:124
#3  0x00007ffff7b8ba4b in ped_file_system_probe_specific (
    fs_type=0x7ffff7dd20c0 <nilfs2_type>, geom=0x61de38) at filesys.c:203
#4  0x00007ffff7b8bc96 in ped_file_system_probe (geom=0x61de38) at filesys.c:273
#5  0x00007ffff7ba3614 in read_table (disk=0x61e1e0, sector=0, is_extended_table=0)
    at dos.c:1050
#6  0x00007ffff7ba3850 in msdos_read (disk=0x61e1e0) at dos.c:1106
#7  0x00007ffff7b8d912 in ped_disk_new (dev=0x61e130) at disk.c:200
#8  0x000000000040764e in do_print (dev=0x7fffffffeb08, diskp=0x7fffffffeb10)
    at parted.c:1067
#9  0x0000000000405346 in command_run (cmd=0x617650, dev=0x7fffffffeb08, 
    diskp=0x7fffffffeb10) at command.c:141
#10 0x000000000040ea27 in non_interactive_mode (dev=0x7fffffffeb08, disk=0x7fffffffeb10, 
    cmd_list=0x6146c0 <commands>, argc=1, argv=0x7fffffffec20) at ui.c:1636
#11 0x000000000040abd2 in main (argc=1, argv=0x7fffffffec20) at parted.c:2295


There were two problems I saw:

1. is_valid_nilfs_sb() should make sure the subtraction bytes - sumoff -
4 won't give a negative number. I saw 10 for bytes and 16 for sumoff and
that was why the len argument to __efi_crc32() was so strange, the
negative number being sent over to an unsigned long.

2. Not sure if you'll want to do this part differently than my patch
does, but nilfs2_probe() should read and allocate enough sectors to hold
a struct nilfs2_super_block.  is_valid_nilfs_sb() will be passing up to
1024 bytes to __efi_crc32(). If only one 512 byte sector had been
allocated with alloca and read from disk that would cause reads off the
the end of the stack even if bytes were more than sumoff - 4. This isn't
the case I saw but I think it would be a problem if s_bytes happened to be
between 508 and 1024.

I've attached a patch and tested it in my vm.  I wanted to get this out
to you before I go away for the weekend, but if you'd like me to try to
write a test I could attempt that next week, perhaps.

Regards,
Mike Small


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=avoid-sigsegv-nilfs2-crc32.patch

From: Michael Small <smallm@HIDDEN>
Date: Fri, 8 Feb 2019 17:01:43 -0500
Subject: [PATCH] Avoid sigsegv in case 2nd nilfs2 superblock magic accidently
 found.

1. is_valid_nilfs_sb: make sure the subtraction bytes - sumoff - 4
won't give a negative number. That as the len argument to
__efi_crc32() would give a very large number for the latter's for
loop limit, since len is unsigned long.

2. nilfs2_probe: Read and allocate enough sectors to hold a
struct nilfs2_super_block.  is_valid_nilfs_sb() will be passing
up to 1024 bytes to __efi_crc32(). If only one 512 byte sector
had been allocated with alloca and read from disk that would cause
reads off the the end of the stack even if bytes were more than
sumoff - 4.

 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libparted/fs/nilfs2/nilfs2.c b/libparted/fs/nilfs2/nilfs2.c
index b42a464..9ad1bfc 100644
--- a/libparted/fs/nilfs2/nilfs2.c
+++ b/libparted/fs/nilfs2/nilfs2.c
@@ -89,7 +89,7 @@ is_valid_nilfs_sb(struct nilfs2_super_block *sb)
 		return 0;
 
 	bytes = PED_LE16_TO_CPU(sb->s_bytes);
-	if (bytes > 1024)
+	if (bytes > 1024 || bytes < sumoff - 4)
 		return 0;
 
 	crc = __efi_crc32(sb, sumoff, PED_LE32_TO_CPU(sb->s_crc_seed));
@@ -113,11 +113,13 @@ nilfs2_probe (PedGeometry* geom)
 	const int sectors = (4096 + geom->dev->sector_size - 1) /
 			     geom->dev->sector_size;
 	char *buf = alloca (sectors * geom->dev->sector_size);
-	void *buff2 = alloca (geom->dev->sector_size);
+	const int sectors2 = sizeof(struct nilfs2_super_block) / geom->dev->sector_size +
+                (sizeof(struct nilfs2_super_block) % geom->dev->sector_size == 0) ? 0 : 1;
+	void *buff2 = alloca (sectors2 * geom->dev->sector_size);
 
 	if (ped_geometry_read(geom, buf, 0, sectors))
 		sb = (struct nilfs2_super_block *)(buf+1024);
-	if (ped_geometry_read(geom, buff2, sb2off, 1))
+	if (ped_geometry_read(geom, buff2, sb2off, sectors2))
 		sb2 = buff2;
 
 	if ((!sb || !is_valid_nilfs_sb(sb)) &&
-- 
2.7.4


--=-=-=--




Acknowledgement sent to Mike Small <smallm@HIDDEN>:
New bug report received and forwarded. Copy sent to bug-parted@HIDDEN. Full text available.
Report forwarded to bug-parted@HIDDEN:
bug#34392; Package parted. Full text available.
Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.
Last modified: Tue, 12 Feb 2019 18:00:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.