Path: csiph.com!eternal-september.org!feeder.eternal-september.org!reader01.eternal-september.org!.POSTED!not-for-mail
From: Keith Thompson <kst-u@mib.org>
Newsgroups: comp.lang.c
Subject: Re: "...the value becomes,indeterminate each time the declaration is reached"
Date: Fri, 01 Feb 2019 12:47:25 -0800
Organization: None to speak of
Lines: 49
Message-ID: <ln7eejw80y.fsf@kst-u.example.com>
References: <q2q79p$1j7$1@dont-email.me> <5c51d4fb.236542547@news.xmission.com> <q2sq30$3u8$1@dont-email.me> <q2vcq9$gem$1@dont-email.me> <20190201132618.6ed57eb670bad5d4400ccbc8@g{oogle}mail.com> <q31df3$pkt$1@dont-email.me> <9MY4E.238695$Y32.184604@fx19.am4> <q3218l$2tr$1@dont-email.me> <initialization-20190201191205@ram.dialup.fu-berlin.de> <q324tg$t9p$1@dont-email.me> <initialization-20190201203311@ram.dialup.fu-berlin.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Injection-Info: reader02.eternal-september.org; posting-host="bfef272611ec40d8078f690171585da1"; logging-data="701"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18l1x8KEvYJM3omABsI0mt6"
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux)
Cancel-Lock: sha1:h6iSiFiRAb9BbAlPZ7Xgrqhw0DM= sha1:B/1h+b8gUMgSsLBW8/J/u575C5g=
Xref: csiph.com comp.lang.c:140462

ram@zedat.fu-berlin.de (Stefan Ram) writes:
[...]
>   This is an uninitialized pointer dereference vulnerability
>   discovered in the Linux kernel and patched in April 2015:
>
> static int queue_manag(void *data)
> {
>     /* back log is defined without initialization */
>     struct crypto_async_request *backlog;
>
>     if (cpg->eng_st == ENGINE_IDLE) {
>         backlog = crypto_get_backlog(&cpg->queue);
>     }
>
>     if (backlog) {
>         /* uninitialized pointer dereferenced! */
>         backlog->complete(backlog, -EINPROGRESS);
>     }
>
>   return 0;
>   }
>
>   A »= 0« is enough to remove the vulnerability in this case.

Yes, and in this case initializing backlog to 0 (I'd use NULL)
is the correct fix.  (Blindly initializing objects to 0 in cases
where that's not a meaningful value is more questionable.)

A sufficiently clever compiler can issue a warning in this case,
by determining that there are execution paths that lead to `backlog`
being dereferenced before a value is assigned to it.  (And in fact
the `if (backlog)` has undefined behavior, though it's not going
to cause problems in this particular environment.)

gcc warns that `backlog` is used uninitialized with "-Wall" along
with "-O1" or higher.  (gcc 4.1.2, from 2008, is able to issue a
warning.)  clang gives a more detailed warning with "-Wall" at all
optimization levels.  I wonder how that bug went unfixed until 2015.

I'm not suggesting that it's ok to write incorrect code because
the compiler will catch it, but compiler warnings and other forms
of static analysis are valuable tools.

[...]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */