linux-kernel - RE: [RFC] tools/nolibc: replace duplicated -ENOSYS return with single -ENOSYS return

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: <20230831074151.7900-1-falcon@tinylab.org>
Date:   Thu, 31 Aug 2023 15:41:51 +0800
From:   Zhangjin Wu <falcon@...ylab.org>
To:     david.laight@...lab.com
Cc:     ammarfaizi2@...weeb.org, arnd@...db.de, falcon@...ylab.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        tanyuan@...ylab.org, thomas@...ch.de, w@....eu
Subject: RE: [RFC] tools/nolibc: replace duplicated -ENOSYS return with single -ENOSYS return

Hi, David, Hi Ammar

> From: Ammar Faizi
> > Sent: 30 August 2023 15:41
> > 
> > On 8/28/23 4:51 AM, David Laight wrote:
> > > I just found a(nother) clang bug:
> > > 	int f(void) { return "a"[2]; }
> > > compiles to just a 'return'.
> > 
> > I don't think that's a bug. It's undefined behavior due to an
> > out-of-bound read. What do you expect it to return?
> 
> I was actually expecting a warning/error if it didn't just read
> the byte after the end of the string.
> 
> Just silently doing nothing didn't seem right for a modern compiler.
>

godbolt.org uses 'orange' color (see its right top) to indicate a
warning, it does report a warning (see Output label in right bottom)
when we access the byte after the end of the string, but gcc doesn't
report a warning ;-)

	int test_outofbound(void)
	{
	    return "a"[10];
	}

see the last test case in https://godbolt.org/z/9jY4xoWrT

But it is safe if we use the trick like David used for the above
__atoi() macro:

    if (str[0]) {  /* always make sure the index is safe and stop at the end of string */
        if (str[1]) {
	    if (str[2]) {
	       ....
	    }
	}
    }

We also need this style of checking for the delta logic in __atoi_add(). have
randomly tried different clang and gcc versions, seems all of them work
correctly, but the compiling speed is not that good if we want to support the
worst cases like "((0x900000 + 0x0f0000) + 5)", the shorter one
"((0x900000+0x0f0000)+5)" is used by ARM+OABI (not supported by nolibc
currently), therefore, we can strip some tailing branches but it is either not
that fast, of course, the other architectures/variants can use faster
__atoi_add() versions with less branches and without hex detection, comparison
and calculating.

As a short summary, the compling speed should not be a big problem for most of
the architectures but to support the worst case __NR_*, the compiling speed
will be very slow (for these cases, perhaps we can use a C version of
atoi_add() instead or convert them to a more generic style: (6000 + 111), no
hex and no multiple add), and the .i output is a little ugly and the debugging
may be also a problem: for we can not assume the kernel developers always
define a short and a simple style of __NR_* as we expected. So, the __nrtoi()
requires more work, let's delay the whole RFC patchset and work on some more
urgent tasks at first as suggested by Willy, but David's NR_toi() prototype is
really a very valuable base for future work, really appreciate, I will back to
this discussion if have any new progress, thanks!

Thanks very much,
Zhangjin

> 	David