NetBSD Problem Report #53831

From www@NetBSD.org  Thu Jan  3 17:20:13 2019
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id B352E7A10D
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  3 Jan 2019 17:20:13 +0000 (UTC)
Message-Id: <20190103172012.78D6F7A1E3@mollari.NetBSD.org>
Date: Thu,  3 Jan 2019 17:20:12 +0000 (UTC)
From: coypu@sdf.org
Reply-To: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Subject: libhfs might be wrong in edge cases
X-Send-Pr-Version: www-1.0

>Number:         53831
>Category:       kern
>Synopsis:       libhfs might be wrong in edge cases
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 03 17:25:00 +0000 2019
>Last-Modified:  Fri Jan 04 04:50:00 +0000 2019
>Originator:     coypu
>Release:        NetBSD 8.99.27
>Organization:
>Environment:
NetBSD planets 8.99.27 NetBSD 8.99.27 (GENERIC) #1: Fri Dec 21 09:30:22 IST 2018  fly@planets:/home/fly/obj/sys/arch/amd64/compile/GENERIC amd64
>Description:
Foreword:

Looking at the code,

HFS_LIBERR is (if you're not PCC, which will print stack garbage)



     65 #define HFS_LIBERR(format, ...) \
     66 	do{ hfslib_error(format, __FILE__, __LINE__, ##__VA_ARGS__); \
     67 		goto error; } while(/*CONSTCOND*/ 0)

In many places, the error case looks like this:

      result = 1;

      if (!test_for_things)
           HFS_LIBERR("something");

error:
       cleanup...
       return result;

For this to function in a sensible way, result must only be set to zero towards the end, not in the middle.




Unfortunately, hfslib_open_volume:


    176 	result = 1;
...

    336 	/*
    337 	 * If this volume uses case-folding comparison and the folding table hasn't
    338 	 * been created yet, do that here. (We don't do this in hfslib_init()
    339 	 * because the table is large and we might never even need to use it.)
    340 	 */
    341 	if (out_vol->keycmp == hfslib_compare_catalog_keys_cf && hfs_gcft == NULL)
    342 		result = hfslib_create_casefolding_table();
    343 	else
    344 		result = 0;
    345 
    346 	/*
    347 	 * Find and store the volume name.
    348 	 */
    349 	if (hfslib_make_catalog_key(HFS_CNID_ROOT_FOLDER, 0, NULL, &rootkey) == 0)
    350 		HFS_LIBERR("could not make root search key");
...

    359 error:
    360 	if (result != 0 && isopen)
    361 		hfslib_close_volume(out_vol, cbargs);
    362 	if (buffer != NULL)
    363 		hfslib_free(buffer, cbargs);
    364 	return result;



Further HFS_LIBERR might be misbehaving, looking like success but failing.
I am not sure this is the case yet, but bringing this to attention.

It might be better practice to have HFS_LIBERR set result to a non-zero value too.
>How-To-Repeat:

>Fix:

>Audit-Trail:
From: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/53831: libhfs might be wrong in edge cases
Date: Thu, 3 Jan 2019 17:38:48 +0000

 Another one of this type that is worth inspecting is:

 hfslib_find_catalog_record_with_key
 ..



     597 	do {
 ...
     611 		for (recnum = 0; recnum < nd.num_recs; recnum++)
     612 		{
 ...
     648 			} else if (keycompare == 0) {
     649 				/* If leaf node, found an exact match. */
     650 				result = 0;
     651 				break;

 ..
     667 	} while (nd.kind != HFS_LEAFNODE);

 This is somewhat risky, because we are in one of two loops, break;
 is not for both of them.

 I am still not sure, just the code choices seemed fishy on a drive-by
 inspection of all src :-)

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: re: kern/53831: libhfs might be wrong in edge cases
Date: Fri, 04 Jan 2019 07:03:28 +1100

 i think this is not a problem.  "result = 1" means "it failed".


 .mrg.

From: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/53831: libhfs might be wrong in edge cases
Date: Fri, 4 Jan 2019 04:49:17 +0000

 Sorry, I'll try to be more clear:

     343 	else
     344 		result = 0;									<- result is set to "success"
     345 
     346 	/*
     347 	 * Find and store the volume name.
     348 	 */
     349 	if (hfslib_make_catalog_key(HFS_CNID_ROOT_FOLDER, 0, NULL, &rootkey) == 0)
     350 		HFS_LIBERR("could not make root search key");					<- erroring here will return "success"

Home	PR Database Search