linux-ext4 - Re: [PATCH v2 10/12] hashmap: change return value type of ext2fs_hashmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YPEDG6sEoq5d4tsv@mit.edu>
Date:   Thu, 15 Jul 2021 23:55:07 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     wuguanghao <wuguanghao3@...wei.com>
Cc:     linux-ext4@...r.kernel.org, artem.blagodarenko@...il.com,
        liuzhiqiang26@...wei.com, linfeilong@...wei.com
Subject: Re: [PATCH v2 10/12] hashmap: change return value type of
 ext2fs_hashmap_add()

On Wed, Jun 30, 2021 at 04:27:22PM +0800, wuguanghao wrote:
> From: Zhiqiang Liu <liuzhiqiang26@...wei.com>
> 
> In ext2fs_hashmap_add(), new entry is allocated by calling
> malloc(). If malloc() return NULL, it will cause a
> segmentation fault problem.
> 
> Here, we change return value type of ext2fs_hashmap_add()
> from void to int. If allocating new entry fails, we will
> return 1, and the callers should also verify the return
> value of ext2fs_hashmap_add().

Changing the function signature of functions in libext2fs, which can
be a shared library, is problematic, since it can potentially break
callers who are depending on the existing shared library ABI.

In this case, making a function returning void return something else
isn't quite so bad, but it still puts callers in a quandry, since they
won't necessarily know if they have linked against an older library
which is not returning an error (or not).

Unfortunately, there's no other way to fix this, and creating a new
ext2fs_hashmap_add2() just to add a return code seems like overkill.
Grumble.

I guess it's OK to do it, since there _probably_ aren't users of
ext2fs_hashmap_add outside of e2fsprogs.  But if we are going to make
this change, we should really have ext2fs_hashmap_add return a
errcode_t, like the other libext2fs functions.

						- Ted