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  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]
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

Powered by blists - more mailing lists