linux-ext4 - Re: [PATCH] ext4: remove unreachable statement inside __es_insert

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] [day] [month] [year] [list]

Date:   Thu, 22 Aug 2019 11:14:44 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Austin Kim <austindh.kim@...il.com>
Cc:     adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext4: remove unreachable statement inside
 __es_insert_extent()

On Thu, Aug 22, 2019 at 03:37:43PM +0900, Austin Kim wrote:
> __es_insert_extent() never returns -EINVAL after BUG is executed.
> So remove unreachable code.
> ---
>  fs/ext4/extents_status.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index a959adc..7f97360 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -781,7 +781,6 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes)
>  			p = &(*p)->rb_right;
>  		} else {
>  			BUG();
> -			return -EINVAL;

This would not be safe in the case of !CONFIG_BUG.  (See init/Kconfig)

It's fair to argue that we shouldn't have CONFIG_BUG --- or
!CONFIG_BUG should still cause the kernel to stop without actually
printing the full BUG information, for those tiny kernel applications
which are really worried about kernel text space.

It also would be fair to argue that we should remove the unreachable
annotation for BUG(), or even, add a *reachable* annotation to catch
code where something something terribly might happen if the kernel is
built with !CONFIG_BUG and we trip against a bug.

But this is a much higher level issue than your sending individual
paches subsystems.

Regards,

						- Ted