netdev - Re: [GIT] Networking

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, 2 Nov 2015 18:38:44 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Andy Lutomirski <luto@...nel.org>,
	David Miller <davem@...emloft.net>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Network Development <netdev@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [GIT] Networking

On Mon, Nov 2, 2015 at 5:58 PM, Andy Lutomirski <luto@...capital.net> wrote:
>
> Based in part on an old patch by Sasha, what if we relied on CSE:
>
> if (mul_would_overflow(size, n))
>   return NULL;
> do_something_with(size * n);

I suspect we wouldn't even have to rely on CSE. Are these things in
performance-critical areas? I suspect our current "use divides" is
actually slower than just using two multiplies, even if one is only
used for overflow checking.

That said, I also think that for something like this, where we
actually have a *reason* for using a special overflow helper function,
we could just try to use the gcc syntax.

I don't think it's wonderful syntax, but at least there's an excuse
for odd/ugly code in those kinds of situations. The reason I hated the
unsigned subtraction case so much was that the simple obvious code
just avoids all those issues entirely, and there wasn't any real
*reason* for the nasty syntax. For multiplication overflow, we'd at
least have a reason.

Sadly, the *nice* syntax, where we'd do something like "goto label"
when the multiply overflows, does not mesh well with inline asm.  Yes,
gcc now has "asm goto", but you can't use it with an output value ;(

But from a syntactic standpoint, the best syntax might actually be
something like

        mul = multiply_with_overflow(size, n, error_label);
        do_something_with(mul);

    error_label:
        return NULL;

and it would *almost* be possible to do this with inline asm if it
wasn't for the annoying "no output values" case.  There are many other
cases where I'd have wanted to do this (ie the whole "fetch value from
user space, but if an exception happens, point the exception handler
at the label).

Back in May, we talked to the gcc people about allowing output values
that are unspecified for the "goto" case (so only a fallthrough would
have them set), but I think that that doesn't match how gcc internally
thinks about branching instructions..

But you could still hide it inside a macro and make it expand to something like

   #define multiply_with_overflow(size, n, error_label) ({
        unsigned long result, error; \
        .. do multiply in asm, set result and error... \
        if (error) goto error_label;
        result; })

which would allow the above kind of odd hand-coded try/catch model in
C. Which I think would be pretty understandable and not very prone to
getting it wrong. Hmm?

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html