linux-kernel - Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants

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]

Date:   Mon, 26 Aug 2019 14:55:48 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Uwe Kleine-König <uwe@...ine-koenig.org>,
        Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants

On (08/24/19 16:58), Andrew Morton wrote:
> On Sun, 25 Aug 2019 01:37:23 +0200 Uwe Kleine-König <uwe@...ine-koenig.org> wrote:
> 
> > 	pr_info("probing failed (%dE)\n", ret);
> > 
> > expands to
> > 
> > 	probing failed (EIO)
> > 
> > if ret holds -EIO (or EIO). This introduces an array of error codes. If
> > the error code is missing, %dE falls back to %d and so prints the plain
> > number.
> 
> Huh.  I'm surprised we don't already have this.  Seems that this will
> be applicable in a lot of places?  Although we shouldn't go blindly
> converting everything in sight - that would risk breaking userspace
> which parses kernel strings.
> 
> Is it really necessary to handle the positive errnos?  Does much kernel
> code actually do that (apart from kernel code which is buggy)?

Good point.
POSIX functions on error usually return -1 (negative value) and set errno
(positive value). Positive errno value can be passed to strerror() or
strerror_r() that decode that value and return a human readable
representation. E.g. strerr(9) returns "Bad file descriptor".

We don't have errno. Instead, and I may be wrong on this, kernel functions
are expected to return negative error codes. A very quick grep shows that
there are, however, patterns like "return positive errno".
E.g. drivers/xen/xenbus/xenbus_xs.c: get_error()

		return EINVAL;

But this EINVAL eventually becomes negative

	err = get_error(ret);
	return ERR_PTR(-err);

or net/bluetooth/lib.c: bt_to_errno(). But, once again, bt_to_errno()
return value eventually becomes negative:

	err = -bt_to_errno(hdev->req_result);

So errstr() probably can handle only negative values. And, may be,
I'd rename errstr() to strerror(); just because there is a well known
function, which "translates" errnos.

Unlike strerror(), errstr() just returns a macro name. Example:
	"Request failed: EJUKEBOX"

EJUKEBOX does not tell me anything. A quick way to find out what does
EJUKEBOX stand for is to grep include/linux/errno.h

#define EJUKEBOX  528  /* Request initiated, but will not complete before timeout */

One still has to grep; either for 528 or for EJUKEBOX. I think that it
might be simpler, however, to grep for EJUKEBOX, because one can grep
the source code immediately, while in case of 528 one has to map 528
to the corresponding macro first and then grep the source code for EJUKEBOX.
Overall %dE looks interesting.

	-ss