krb5 database operations enter infinite loop

Bug #1347147 reported by Benjamin Kaduk
30
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Kerberos
Unknown
Unknown
gcc
Fix Released
High
gcc-4.8 (Ubuntu)
Fix Released
Undecided
Unassigned
Trusty
Fix Released
Undecided
Unassigned
gcc-4.9 (Ubuntu)
Fix Released
Undecided
Unassigned
krb5 (Ubuntu)
Fix Released
High
Unassigned
Trusty
Fix Released
High
Sam Hartman

Bug Description

[Impact]

On krb5 KDC databases with more than a few hundred principals, operations can enter an infinite loop in the database library. This affects both read and write operations. If operators are fortunate, they will encounter this bug while testing a migration. If they are not so fortunate, they will encounter this bug in a production KDC when the number of principals crosses the threshold where this bug manifests, resulting in a service outage and possible database corruption. Probably the only way to restore service in that situation is to install a patched KDC or to downgrade to an unaffected version.

Both Trusty and Utopic amd64 have been verified to have this issue.

One concrete reported example is an invocation of kdb5_util load (as part of a slave KDC propagation) spinning:

http://mailman.mit.edu/pipermail/kerberos/2014-July/020007.html

Additional failure modes are likely

A branch is linked including the upstream work around for this bug, along with two other patches to bugs already nominated for trusty applied to the krb5 in trusty.

For utopic, the simplest fix is to rebuild krb5 with the compiler currently in utopic. An alternative is to request that the Debian maintainers (both monitoring this bug for such a request) upload the upstream work around to Debian and sync that. You could do an ubuntu-specific upload but it seems undesirable to introduce a change between Ubuntu and Debian when all the right parties are happy to avoid it.

The upstream patch works around a compiler optimizer bug in the gcc-4.8 series, which incorrectly deduces that a strict aliasing violation has occurred and miscompiles part of the bundled libdb2 library that the KDC database back end depends upon. The miscompilation causes a data structure to contain an inappropriate cycle, which leads to an infinite loop when the structure is traversed.

[Test Case]

apt-get install krb5-kdc krb5-admin-server
kdb5_util -W -r T create -s
awk 'BEGIN{ for (i = 0; i < 1024; i++) { printf("ank -randkey a%06d\n", i) } }' /dev/null | kadmin.local -r T

(Enter any password for the master key when requested.)

On platforms with this issue, kadmin.local spins consuming 100% CPU after a few hundred principals have been created. (This is "a000762" on two examples.)

To clean up,

rm /etc/krb5kdc/principal*

or

krb5kdc -r T destroy

but the latter can possibly enter the same infinite loop.

[Regression Potential]

Negligible.

It is theoretically possible that our upstream workaround, which involves using TAILQ macros instead of CIRCLEQ macros in the bundled libdb2 that backs the KDC database, will have some as-yet undiscovered bugs or compiler interactions with consequences worse than this current issue. I think this is rather unlikely.

The patched libdb2 passes both the extensive libdb2 test suite and the rest of the krb5 test suite. Prior to patching, compiling krb5 with an affected gcc would cause the krb5 test suite to stall when it reached the libdb2 test suite. (The test suite stall is how we became aware of the gcc optimizer bug.)

The BSD TAILQ macros are generally considered to be safer than the CIRCLEQ macros, and the various open-source BSD derivatives have made the corresponding change to their libdb sources years ago, with no reported ill effects that I can see.

Original report from Ben Kaduk:

==========

In some conditions, propagating a kerberos database to a slave KDC server can stall.
This is due to a misoptimization by gcc 4.8 of the CIRCLEQ famliy of macros, apparently due to overzealous strict aliasing deductions.

One case of this stall is reported at http://mailman.mit.edu/pipermail/kerberos/2014-July/020007.html (and the rest of the thread), and there is an entry in the upstream bugtracker at http://krbdev.mit.edu/rt/Ticket/Display.html?id=7860 .

gcc 4.9 (as used in Debian unstable at present) is not believed to induce this problem. Upstream has patched their code to use the TAILQ family of macros instead, as a workaround, but that workaround has not yet appeared in an upstream release: https://github.com/krb5/krb5/commit/26d8744129

Because of the different compiler versions used on Debian and Ubuntu, I am filing this as an Ubuntu-specific bug.

Revision history for this message
Taylor Yu (tlyu) wrote :

Test case:

On Ubuntu 14.04 on amd64, install krb5-admin-server and krb5-kdc.

kdb5_util -W -r T create -s
awk 'BEGIN{ for (i = 0; i < 1024; i++) { printf("ank -randkey a%06d\n", i) } }' /dev/null | kadmin.local -r T

For me, kadmin.local begins consuming nearly 100% CPU starting at "a000762". This implies that sites with more than a few hundred principals are likely to find running a KDC using the 14.04 krb5 packages impossible.

Changed in krb5 (Ubuntu):
status: New → Confirmed
Revision history for this message
Anders Kaseorg (andersk) wrote :

Reproduced on current utopic amd64, with the same results, kadmin.local spinning at 100% CPU on a000762.

tags: added: regression-release testcase trusty utopic
Revision history for this message
Anders Kaseorg (andersk) wrote :

Note that krb5 in utopic amd64 was compiled against gcc 4.9.0-10ubuntu2 (build log: https://launchpadlibrarian.net/179748030/buildlog_ubuntu-utopic-amd64.krb5_1.12.1%2Bdfsg-3ubuntu1_UPLOADING.txt.gz). So it looks like it might be wrong to believe that GCC 4.9 does not induce this problem.

Revision history for this message
Anders Kaseorg (andersk) wrote :

However, the problem seems to go away after locally recompiling krb5 1.12.1+dfsg-3ubuntu1 with gcc-4.9 4.9.1-3ubuntu2.

(This could still indicate any number of things, though.)

Revision history for this message
Anders Kaseorg (andersk) wrote :

Sorry, I read the build log incorrectly in comment #3. krb5 in utopic amd64 was actually compiled against gcc-4.8 4.8.3-4ubuntu2. So the belief may still be valid.

(I was misled because the metapackage gcc 4:4.9.0-3ubuntu5 actually installed a symlink to gcc-4.8, despite the version number.)

Revision history for this message
Anders Kaseorg (andersk) wrote :

I’ve written a test case that clearly demonstrates the GCC 4.8 bug responsible for the kadmin.local failure:

$ gcc-4.8 -Wall -O2 bug.c -o bug
$ ./bug
$ echo $?
1
$ gcc-4.9 -Wall -O2 bug.c -o bug
$ ./bug
$ echo $?
0

A git bisection of the GCC source shows that this bug disappeared in https://gcc.gnu.org/r202525. However, I don’t understand why; the fact that r202525 was intended to fix a “missed-optimization” bug (https://gcc.gnu.org/PR58404) is a little bit terrifying.

Revision history for this message
In , Anders Kaseorg (andersk) wrote :

Kerberos is miscompiled by gcc-4.8. The impact is detailed at https://bugs.launchpad.net/bugs/1347147, but here is a reduced test case. The expected return is 0, but when compiled with gcc-4.8 -O2, it returns 1.

$ cat bug.c

struct node { struct node *next, *prev; } node;
struct head { struct node *first; } heads[5];
int k = 2;
struct head *head = &heads[2];

int main()
{
  node.prev = (void *)head;
  head->first = &node;

  struct node *n = head->first;
  struct head *h = &heads[k];

  if (n->prev == (void *)h)
    h->first = n->next;
  else
    n->prev->next = n->next;

  n->next = h->first;
  return n->next == &node;
}

$ gcc-4.7 -Wall -O2 bug.c -o bug; ./bug; echo $?
0
$ gcc-4.8 -Wall -O2 bug.c -o bug; ./bug; echo $?
1
$ gcc-4.9 -Wall -O2 bug.c -o bug; ./bug; echo $?
0
$ dpkg -l gcc-4.7 gcc-4.8 gcc-4.9
[…]
ii gcc-4.7 4.7.4-2ubuntu1 amd64 GNU C compiler
ii gcc-4.8 4.8.3-6ubuntu1 amd64 GNU C compiler
ii gcc-4.9 4.9.1-3ubuntu2 amd64 GNU C compiler

I bisected the point where the problem disappeared between 4.8 and 4.9 at r202525. However, I don’t understand why. I’m scared by the fact that r202525 was intended to fix a “missed-optimization” bug (bug 58404).

Revision history for this message
In , Rguenth (rguenth) wrote :

The testcase is violating strict-aliasing rules as you access a struct head
as struct node here:

  if (n->prev == (void *)h)
    h->first = n->next;
  else
    n->prev->next = n->next;

as n->prev points to &heads[0] while h is &heads[2] (an out-of-bound pointer).
So n->prev is a struct head and you access a next field of a struct node of it.

Changing k to 0 makes the testcase pass (now you don't run into the bogus
path).

Revision history for this message
In , Greg Hudson (ghudson) wrote :

How do you conclude that n->prev points to &heads[0]? node.prev receives the value (void *)head, where head is initialized to &heads[2]. I cannot see any uses of &heads[0] in the test program.

Revision history for this message
In , Anders Kaseorg (andersk) wrote :

(In reply to Richard Biener from comment #1)
> The testcase is violating strict-aliasing rules as you access a struct head
> as struct node here:

Agree with ghudson here: n->prev points to &heads[2], not &heads[0].

Although assigning a casted struct head * to a struct node * is arguably sloppy, the standard does not prohibit it, as long as it is never dereferenced in that form.

Revision history for this message
Robie Basak (racb) wrote :

Thank you for taking the time to report this bug and helping to make Ubuntu better.

I don't follow all of the conversation here. Is it clear that the workaround suggested (https://github.com/krb5/krb5/commit/26d8744129) is still valid, should be applied to the version of krb5 in Utopic, will fix the version in Utopic, won't introduce any regression, and will be released by upstream? If someone can confirm these things, then we can get it landed in Ubuntu.

Next, for Trusty, we need the steps in https://wiki.ubuntu.com/StableReleaseUpdates#Procedure followed - in particular, a test case that has exact steps to reproduce the problem with a slave KDC so that the problem can be verified fixed with the new proposed binary during stable verification testing, and an understanding of impact to users and potential regression risk so that the SRU team can make a decision about whether this fix is acceptable to SRU to Trusty.

I understand that this is potentially difficult to reproduce and verify. This is fine and we can be pragmatic about it, but if this is the case then extra clarity around these issues would be appreciated. For verification, we'll still want to exercise the code around the areas changed to make sure that we haven't fundamentally broken anything, and so a test case is still useful even if it comes with the caveat that it may not always fail.

Setting to Triaged since upstream consider this a valid bug, and that's good enough for me. I've not set Importance because it isn't clear to me yet what proportion of production krb5-kdc slave users are or would actually be affected.

Changed in krb5 (Ubuntu):
status: Confirmed → Triaged
Revision history for this message
Robie Basak (racb) wrote :

22:56 <tlyu> rbasak: do you consider the current test case (comment #1) inadequate?
23:01 <rbasak> tlyu: I'm sorry. That test case is fine. I missed it when writing my comment.

Revision history for this message
Benjamin Kaduk (kaduk-launchpad) wrote :

The workaround suggested (https://github.com/krb5/krb5/commit/26d8744129) is still valid, and appears on upstream's 1.12 release branch already (as https://github.com/krb5/krb5/commit/c7bb9278ad12c9). It will appear in the 1.12.2 release. Furthermore, I have applied it to the Debian packaging as http://anonscm.debian.org/cgit/pkg-k5-afs/debian-krb5-2013.git/commit/?id=3d3bf0c075af62f278b6 (though it has not yet been uploaded to Debian proper). This workaround should not introduce any regressions.

I should also note that, if I understand correctly, we expect a no-change rebuild of krb5 using the gcc currently in utopic to also be a valid workaround. However, the gcc in trusty is still expected to produce krb5 packages which exhibit this bug.

Revision history for this message
Sam Hartman (hartmans) wrote : Re: [Bug 1347147] Re: krb5 database propagation enters infinite loop

I'm happy to upload a new krb5 to debian so you can sync it if you want
that approach.
I'm also happy if Ubuntu wants to go with a binary rebuild of krb5.

Revision history for this message
Sam Hartman (hartmans) wrote :

Please see https://launchpad.net/~hartmans/+archive/ubuntu/krb5 for
trusty packages that should fix the problem.

Can I get confirmation from Tom or someone else that without these
packages trusty fails the reproduce test in comment #1 and with them, it
succeeds the test proposed in comment #1?

I'm updating a branch I have for proposed trustry krb5 updates
(lp:~hartmans/ubuntu/trusty/krb5/gss-infinite-loop) to include this
patch.

Revision history for this message
Sam Hartman (hartmans) wrote :

I'm sorry, can I get someone to test the packages at
https://launchpad.net/~hartmans/+archive/ubuntu/ubuntu-fixes
not the URI I gave in the previous message.
I pulled the wrong PPA off my home page.

Revision history for this message
Taylor Yu (tlyu) wrote : Re: krb5 database propagation enters infinite loop

I confirm that the packages at https://launchpad.net/~hartmans/+archive/ubuntu/ubuntu-fixes appear to fix the problem for Trusty amd64, based on the test case in comment #1.

Revision history for this message
In , Anders Kaseorg (andersk) wrote :

Another bisect between 4.7 and 4.8 shows that the bug appeared with r189321 (bug 52009).

My test case has triggers the bug in more versions than Kerberos does: as far as I can tell, Kerberos was unaffected until r192604.

Revision history for this message
In , Mikpelinux (mikpelinux) wrote :

I've been staring as this test case, and I cannot find any dereference of a wrong-typed pointer value. The only oddity I can find is that at

  if (n->prev == (void *)h)

n == &node, n->prev == (struct node *)&heads[2] (so wrong-typed), h == &heads[2], so there is a '==' being applied to a wrong-typed pointer. Is that undefined behaviour? I'll note that changing the test to

  if ((void *)n->prev == (void *)h)

still reproduces the wrong-code while looking technically Ok.

Also, there is no out-of-bounds error.

Revision history for this message
In , Andreas Schwab (schwab-linux-m68k) wrote :

Equality test against pointer to void is explicitly allowed by the standard and implicitly converts the other pointer to void*.

Revision history for this message
In , Rguenth (rguenth) wrote :

(In reply to Anders Kaseorg from comment #4)
> Another bisect between 4.7 and 4.8 shows that the bug appeared with r189321
> (bug 52009).
>
> My test case has triggers the bug in more versions than Kerberos does: as
> far as I can tell, Kerberos was unaffected until r192604.

Thanks - that pin-points it. tail-merging concludes that

  <bb 3>:
  _11 = n_7->next;
  MEM[(struct head *)_10].first = _11;
  goto <bb 5>;

and

  <bb 4>:
  _13 = n_7->next;
  _10->next = _13;

are equivalent. But they are not - the stores are performed using
different alias sets.

Note that the actual miscompile happens during RTL instruction scheduling.

In 4.9 and trunk tail-merging is faced with

  <bb 3>:
  _11 = n_7->next;
  MEM[(struct head *)&heads][k.1_8].first = _11;
  goto <bb 5>;

  <bb 4>:
  _13 = n_7->next;
  _10->next = _13;

instead but I bet the issue is still there.

So it simply does (on the 4.8 branch):

    case GIMPLE_ASSIGN:
      lhs1 = gimple_get_lhs (s1);
      lhs2 = gimple_get_lhs (s2);
      if (TREE_CODE (lhs1) != SSA_NAME
          && TREE_CODE (lhs2) != SSA_NAME)
        return (vn_valueize (gimple_vdef (s1))
                == vn_valueize (gimple_vdef (s2)));

which shows that we value-number the VDEFs the same.

IMHO VDEF value-numbering is dangerous here.

I have a patch.

Revision history for this message
In , Vries (vries) wrote :

Using this patch on the example from the description field, I can modify the example on the command line:
...
$ diff -u bug-orig.c bug-mod.c
--- bug-orig.c 2014-07-31 14:00:50.663275717 +0200
+++ bug-mod.c 2014-07-31 14:01:49.403276412 +0200
@@ -11,7 +11,7 @@
   struct node *n = head->first;
   struct head *h = &heads[k];

- if (n->prev == (void *)h)
+ if (FORCE n->prev == (void *)h)
     h->first = n->next;
   else
     n->prev->next = n->next;
...

1. -DFORCE="" gives the original
2. -DFORCE="1 ||" forces the condition to true
3. -DFORCE="0 &&" forces the confition to false

In this experiment, we don't use tree-tail-merge:
...
$ gcc -DFORCE="1 ||" bug-mod.c -O2 -fno-strict-aliasing -fno-tree-tail-merge && ./a.out ; echo $?
0
$ gcc -DFORCE="1 ||" bug-mod.c -O2 -fstrict-aliasing -fno-tree-tail-merge && ./a.out ; echo $?
0
$ gcc -DFORCE="0 &&" bug-mod.c -O2 -fno-strict-aliasing -fno-tree-tail-merge && ./a.out ; echo $?
0
$ gcc -DFORCE="0 &&" bug-mod.c -O2 -fstrict-aliasing -fno-tree-tail-merge && ./a.out ; echo $?
1
...

The last result seems to suggest that the example is not type-safe.

My understanding is that the problem is in the line:
  n->prev->next = n->next;
where we effectively do:
  /* ((struct node*)&heads[2])->next = node.next */
which is type-unsafe.

Revision history for this message
In , Rguenther-suse (rguenther-suse) wrote :

On Thu, 31 Jul 2014, vries at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61964
>
> vries at gcc dot gnu.org changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> CC| |vries at gcc dot gnu.org
>
> --- Comment #8 from vries at gcc dot gnu.org ---
> Using this patch on the example from the description field, I can modify the
> example on the command line:
> ...
> $ diff -u bug-orig.c bug-mod.c
> --- bug-orig.c 2014-07-31 14:00:50.663275717 +0200
> +++ bug-mod.c 2014-07-31 14:01:49.403276412 +0200
> @@ -11,7 +11,7 @@
> struct node *n = head->first;
> struct head *h = &heads[k];
>
> - if (n->prev == (void *)h)
> + if (FORCE n->prev == (void *)h)
> h->first = n->next;
> else
> n->prev->next = n->next;
> ...
>
> 1. -DFORCE="" gives the original
> 2. -DFORCE="1 ||" forces the condition to true
> 3. -DFORCE="0 &&" forces the confition to false
>
> In this experiment, we don't use tree-tail-merge:
> ...
> $ gcc -DFORCE="1 ||" bug-mod.c -O2 -fno-strict-aliasing -fno-tree-tail-merge &&
> ./a.out ; echo $?
> 0
> $ gcc -DFORCE="1 ||" bug-mod.c -O2 -fstrict-aliasing -fno-tree-tail-merge &&
> ./a.out ; echo $?
> 0
> $ gcc -DFORCE="0 &&" bug-mod.c -O2 -fno-strict-aliasing -fno-tree-tail-merge &&
> ./a.out ; echo $?
> 0
> $ gcc -DFORCE="0 &&" bug-mod.c -O2 -fstrict-aliasing -fno-tree-tail-merge &&
> ./a.out ; echo $?
> 1
> ...
>
> The last result seems to suggest that the example is not type-safe.
>
> My understanding is that the problem is in the line:
> n->prev->next = n->next;
> where we effectively do:
> /* ((struct node*)&heads[2])->next = node.next */
> which is type-unsafe.

But that line is never executed at runtime (well, unless tail
merging comes along and makes it the only version present).

Richard.

Revision history for this message
In , Rguenth (rguenth) wrote :

Author: rguenth
Date: Thu Jul 31 14:06:59 2014
New Revision: 213375

URL: https://gcc.gnu.org/viewcvs?rev=213375&root=gcc&view=rev
Log:
2014-07-31 Richard Biener <email address hidden>

 PR tree-optimization/61964
 * tree-ssa-tail-merge.c (gimple_equal_p): Handle non-SSA LHS solely
 by structural equality.

 * gcc.dg/torture/pr61964.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr61964.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-tail-merge.c

Revision history for this message
In , Rguenth (rguenth) wrote :

Fixed on trunk sofar.

Revision history for this message
Taylor Yu (tlyu) wrote :

This problem is broader than slave KDCs; it can potentially affect any write operation on a KDC with sufficiently many (more than a few hundred) principals, causing database corruption or denial of service. Altering the test case to create one principal per invocation of kadmin.local shows that the spin condition depends on database contents rather than process memory history.

It also manifests during krb5_db_get_principal(), not just krb5_db_put_principal(), as shown in the below stack trace. Note the krb5_db_get_principal() call in the stack trace is the one that is meant to verify the master key.

Altered test case showing the spin condition on a fresh kadmin.local invocation:

kdb5_util -W -r T create -s
awk 'BEGIN { for (i = 0; i < 1024; i++) { printf("%06d\n", i) } }' /dev/null | (set -e; while read p; do kadmin.local -r T -q "ank -randkey $p"; done)

I still recommend preferring the test case I gave in comment #1 because it executes more quickly.

kadmin.local stack trace:

(gdb) bt
#0 0x00007f3fa70dbcbc in ?? ()
   from /usr/lib/x86_64-linux-gnu/krb5/plugins/kdb/db2.so
#1 0x00007f3fa70d90bc in ?? ()
   from /usr/lib/x86_64-linux-gnu/krb5/plugins/kdb/db2.so
#2 0x00007f3fa70d7bc9 in ?? ()
   from /usr/lib/x86_64-linux-gnu/krb5/plugins/kdb/db2.so
#3 0x00007f3fa70d0ab6 in ?? ()
   from /usr/lib/x86_64-linux-gnu/krb5/plugins/kdb/db2.so
#4 0x00007f3fa70d1bf4 in ?? ()
   from /usr/lib/x86_64-linux-gnu/krb5/plugins/kdb/db2.so
#5 0x00007f3fa79d0047 in krb5_db_get_principal ()
   from /usr/lib/x86_64-linux-gnu/libkdb5.so.7
#6 0x00007f3fa79d365b in ?? () from /usr/lib/x86_64-linux-gnu/libkdb5.so.7
#7 0x00007f3fa79d02c0 in krb5_db_fetch_mkey_list ()
   from /usr/lib/x86_64-linux-gnu/libkdb5.so.7
#8 0x00007f3fa9140f78 in kdb_init_master ()
   from /usr/lib/x86_64-linux-gnu/libkadm5srv_mit.so.9
#9 0x00007f3fa9141e90 in kadm5_init ()
   from /usr/lib/x86_64-linux-gnu/libkadm5srv_mit.so.9
#10 0x0000000000404659 in ?? ()
#11 0x0000000000402bbc in ?? ()
#12 0x00007f3fa8263ec5 in __libc_start_main (main=0x402b70, argc=5,
    argv=0x7fff76716738, init=<optimized out>, fini=<optimized out>,
    rtld_fini=<optimized out>, stack_end=0x7fff76716728) at libc-start.c:287
#13 0x0000000000402c96 in ?? ()

Taylor Yu (tlyu)
summary: - krb5 database propagation enters infinite loop
+ krb5 database operations enter infinite loop
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in gcc-4.8 (Ubuntu):
status: New → Confirmed
Sam Hartman (hartmans)
description: updated
Taylor Yu (tlyu)
description: updated
Revision history for this message
Sam Hartman (hartmans) wrote : Review of Bug 1347147 for nomination for a fix for trusty krb5

hi.
If I'm understanding the SRU procedure correctly,
I think we need to get someone to review the referenced bug for
inclusion in trusty.

https://bugs.launchpad.net/gcc/+bug/1347147

Revision history for this message
Sam Hartman (hartmans) wrote : Re: [Bug 1347147] Review of Bug 1347147 for nomination for a fix for trusty krb5

>>>>> "Sam" == Sam Hartman <email address hidden> writes:

    Sam> hi. If I'm understanding the SRU procedure correctly, I think
    Sam> we need to get someone to review the referenced bug for
    Sam> inclusion in trusty.

Sorry, launchpad strips more mail headers than I thought it did.
That was sent to ubuntu-bugcontrol, cc'd to the bug.

Revision history for this message
Taylor Yu (tlyu) wrote :

Edited description to include text from Sam that was omitted when we crossed edits.

description: updated
Revision history for this message
In , Vries (vries) wrote :

Created attachment 33220
Alternative patch

> But that line is never executed at runtime (well, unless tail
> merging comes along and makes it the only version present).

Ah, right, we consider a program with dead type-unsafe code valid.

This follow-up patch attempts to fix things less conservatively on trunk. Shall I put this through testing or do you see a problem with this approach?

Furthermore, I suspect that a similar issue exists for loads, I'll look into that.

Revision history for this message
Sam Hartman (hartmans) wrote :

debdiff included

Revision history for this message
Thomas Ward (teward) wrote :

Nomination added for krb5 for Trusty per an email thread to bugcontrol. It added for gcc as well but the email chain points to a SRU fix for krb5 and not the compiler at this time.

Revision history for this message
In , Rguenth (rguenth) wrote :

(In reply to vries from comment #12)
> Created attachment 33220 [details]
> Alternative patch
>
> > But that line is never executed at runtime (well, unless tail
> > merging comes along and makes it the only version present).
>
> Ah, right, we consider a program with dead type-unsafe code valid.
>
> This follow-up patch attempts to fix things less conservatively on trunk.
> Shall I put this through testing or do you see a problem with this approach?

Hum. I don't like guarding optimizations with !flag_strict_aliasing, that is,
-fno-strict-aliasing shouldn't get us more optimization.

Also on trunk I'd like to rip out the use of the SCCVN lattice from
tail-merging as there FRE/PRE value-replace every SSA name which means
we don't need it. The tight entanglement between PRE and tail-merge has
given me more headaches recently.

> Furthermore, I suspect that a similar issue exists for loads, I'll look into
> that.

I don't think so.

Revision history for this message
In , Rguenth (rguenth) wrote :

Author: rguenth
Date: Fri Aug 1 07:36:16 2014
New Revision: 213404

URL: https://gcc.gnu.org/viewcvs?rev=213404&root=gcc&view=rev
Log:
2014-08-01 Richard Biener <email address hidden>

 PR tree-optimization/61964
 * tree-ssa-tail-merge.c (gimple_equal_p): Handle non-SSA LHS solely
        by structural equality.

 * gcc.dg/torture/pr61964.c: New testcase.
 * gcc.dg/pr51879-18.c: XFAIL.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.dg/torture/pr61964.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/gcc.dg/pr51879-18.c
    branches/gcc-4_9-branch/gcc/tree-ssa-tail-merge.c

Revision history for this message
In , Rguenth (rguenth) wrote :

Author: rguenth
Date: Fri Aug 1 07:40:01 2014
New Revision: 213405

URL: https://gcc.gnu.org/viewcvs?rev=213405&root=gcc&view=rev
Log:
2014-08-01 Richard Biener <email address hidden>

 PR tree-optimization/61964
 * tree-ssa-tail-merge.c (gimple_operand_equal_value_p): New
 function merged from trunk.
 (gimple_equal_p): Handle non-SSA LHS solely by structural
 equality.

 * gcc.dg/torture/pr61964.c: New testcase.
 * gcc.dg/pr51879-18.c: XFAIL.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr61964.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/pr51879-18.c
    branches/gcc-4_8-branch/gcc/tree-ssa-tail-merge.c

Revision history for this message
Anders Kaseorg (andersk) wrote :

Upstream says this bug still exists in GCC 4.9, even though Kerberos happens to be unaffected there. They have committed a patch to GCC trunk <https://gcc.gnu.org/r213375>, GCC 4.9 <https://gcc.gnu.org/r213404> and for GCC 4.8 <https://gcc.gnu.org/r213405>.

I verified that GCC 4.8 with r213405 fixes the kadmin.local test.

Revision history for this message
In , Rguenth (rguenth) wrote :

Fixed.

Revision history for this message
In , Anders Kaseorg (andersk) wrote :

Thanks. I verified that GCC 4.8 r213405 fixes my test case and the original Kerberos problem.

no longer affects: gcc-4.9 (Ubuntu Trusty)
Revision history for this message
Anders Kaseorg (andersk) wrote :

gcc-4.8 (4.8.3-7ubuntu1) utopic; urgency=medium

  * Merge with Debian; remaining changes:
    - Build from the upstream source.

 -- Matthias Klose <email address hidden> Fri, 01 Aug 2014 12:25:27 +0200

gcc-4.8 (4.8.3-7) unstable; urgency=medium

  * Update to SVN 20140801 (r213447) from the gcc-4_8-branch.
    - CVE-2014-5044, fix integer overflows in array allocation in libgfortran.
      Closes: #756325.
    - Fix PR tree-optimization/61964. LP: #1347147.
  * Fix java.security symlink. Addresses: #756484.

 -- Matthias Klose <email address hidden> Fri, 01 Aug 2014 12:07:59 +0200

I verified that compiling with gcc-4.8 4.8.3-7ubuntu1 in utopic-proposed fixes the problem.

Changed in gcc-4.8 (Ubuntu):
status: Confirmed → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gcc-4.8 - 4.8.3-7ubuntu1

---------------
gcc-4.8 (4.8.3-7ubuntu1) utopic; urgency=medium

  * Merge with Debian; remaining changes:
    - Build from the upstream source.

gcc-4.8 (4.8.3-7) unstable; urgency=medium

  * Update to SVN 20140801 (r213447) from the gcc-4_8-branch.
    - CVE-2014-5044, fix integer overflows in array allocation in libgfortran.
      Closes: #756325.
    - Fix PR tree-optimization/61964. LP: #1347147.
  * Fix java.security symlink. Addresses: #756484.
 -- Matthias Klose <email address hidden> Fri, 01 Aug 2014 12:25:27 +0200

Changed in gcc-4.8 (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gcc-4.9 - 4.9.1-4ubuntu2

---------------
gcc-4.9 (4.9.1-4ubuntu2) utopic; urgency=medium

  * Update to SVN 20140802 (r213510) from the gcc-4_9-branch.
    - Fix PR tree-optimization/61964. LP: #1347147.
  * Fix libphobos cross build.
 -- Matthias Klose <email address hidden> Sat, 02 Aug 2014 02:44:59 +0200

Changed in gcc-4.9 (Ubuntu):
status: New → Fix Released
Robie Basak (racb)
Changed in krb5 (Ubuntu):
importance: Undecided → High
Changed in krb5 (Ubuntu Trusty):
importance: Undecided → High
status: New → Triaged
Revision history for this message
In , Vries (vries) wrote :

(In reply to Richard Biener from comment #13)
> (In reply to vries from comment #12)
> > Furthermore, I suspect that a similar issue exists for loads, I'll look into
> > that.
>
> I don't think so.

How about PR62004 ?

Revision history for this message
Sam Hartman (hartmans) wrote :

I've request a krb5 sync from debian unstable in https://bugs.launchpad.net/bugs/1352438 that should fix this issue and include some needed security fixes in utopic.

Changed in gcc:
importance: Unknown → High
status: Unknown → Fix Released
Revision history for this message
Anders Kaseorg (andersk) wrote :

I verified that this is fixed in utopic-proposed by krb5 1.12.1+dfsg-6.

krb5 (1.12.1+dfsg-6) unstable; urgency=medium

  [ Benjamin Kaduk ]
  * Apply upstream's patch to switch to TAILQ macros instead of CIRCLEQ macros,
    to work around an issue with certain gcc versions. This is expected to
    resolve Ubuntu bug (LP: #1347147).

  [ Sam Hartman ]
  * Include a quick and dirty patch so we build cleanly with -O3 fixing
    incorrect may be uninitialized warnings.

 -- Benjamin Kaduk <email address hidden> Tue, 29 Jul 2014 17:05:37 -0400

krb5 (1.12.1+dfsg-5) unstable; urgency=high

  * Apply upstream patches for CVE-2014-4343, CVE-2014-4344, Closes: #755520,
    Closes: #755521

 -- Benjamin Kaduk <email address hidden> Mon, 21 Jul 2014 17:27:10 -0400

krb5 (1.12.1+dfsg-4) unstable; urgency=high

  * Apply upstream patch for CVE-2014-4341, CVE-2014-4342, Closes: #753624,
    Closes: #753625

 -- Benjamin Kaduk <email address hidden> Fri, 11 Jul 2014 13:43:19 -0400

Changed in krb5 (Ubuntu):
status: Triaged → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.12.1+dfsg-6

---------------
krb5 (1.12.1+dfsg-6) unstable; urgency=medium

  [ Benjamin Kaduk ]
  * Apply upstream's patch to switch to TAILQ macros instead of CIRCLEQ macros,
    to work around an issue with certain gcc versions. This is expected to
    resolve Ubuntu bug (LP: #1347147).

  [ Sam Hartman ]
  * Include a quick and dirty patch so we build cleanly with -O3 fixing
    incorrect may be uninitialized warnings.

 -- Benjamin Kaduk <email address hidden> Tue, 29 Jul 2014 17:05:37 -0400

Changed in krb5 (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in gcc-4.8 (Ubuntu Trusty):
status: New → Confirmed
Revision history for this message
Sam Hartman (hartmans) wrote :

Here's an ubdated debdiff that includes the security update applied to trusty. I'm still waiting for a sponsor for this.

Revision history for this message
Robie Basak (racb) wrote :

Thanks Sam. I'm sorry I can't sponsor krb5, only triage the bug and guide it through to sponsorship. It looks like you know what you're doing here, so I guess we'll just need to wait for a sponsor to look at it. I can see that it's in the queue and working its way up.

Revision history for this message
Sam Hartman (hartmans) wrote : Re: [Bug 1347147] Re: krb5 database operations enter infinite loop

>>>>> "Robie" == Robie Basak <email address hidden> writes:

    Robie> Thanks Sam. I'm sorry I can't sponsor krb5, only triage the
    Robie> bug and guide it through to sponsorship. It looks like you
    Robie> know what you're doing here, so I guess we'll just need to
    Robie> wait for a sponsor to look at it. I can see that it's in the
    Robie> queue and working its way up.

Actually, your reassurance that we've done the right things process wise
is really helpful.
I've been involved in Debian for over 10 years but haven't done a huge
bunch of stuff with Ubuntu so I'm mostly just reading the wikis and
trying to figure it out:-)

Revision history for this message
Duncan White (d-white) wrote :

Hi, I'm from the original organisation (doc.ic.ac.uk) with the kdc5_util load problem on Trusty,
I appreciate all your work on fixing this bug. If it's useful info, I can confirm that taking the
krb5 (1.12.1+dfsg-7) source package [revised version of 1.12.1+dfsg-6 mentioned above] from
Utopic and locally backporting it onto Trusty, and installing the relevant krb5 packages onto
our experimental slave KDC does fix the original kprop/kdc5_util load problem. So I'm temporarily
using that until proper Trusty packages come out.

Revision history for this message
Anders Kaseorg (andersk) wrote :

A second patch was committed a few days ago for another case of the GCC bug (https://gcc.gnu.org/PR62004).

trunk: https://gcc.gnu.org/r213970
4.9: https://gcc.gnu.org/r214044
4.8: https://gcc.gnu.org/r214067

Revision history for this message
Iain Lane (laney) wrote :

Thanks Sam, I've uploaded krb5.

Changed in krb5 (Ubuntu Trusty):
status: Triaged → In Progress
assignee: nobody → Sam Hartman (hartmans)
Revision history for this message
Sam Hartman (hartmans) wrote :

>>>>> "Iain" == Iain Lane <email address hidden> writes:

    Iain> Thanks Sam, I've uploaded krb5. ** Changed in: krb5 (Ubuntu
    Iain> Trusty) Status: Triaged => In Progress

Hi.
I haven't seen this hit proposed yet.
Is that expected? What is the next step?

Revision history for this message
Anders Kaseorg (andersk) wrote :
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Benjamin, or anyone else affected,

Accepted krb5 into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/krb5/1.12+dfsg-2ubuntu5 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in krb5 (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Taylor Yu (tlyu) wrote :

Confirmed that 1.12+dfsg-2ubuntu5 from trusty-proposed fixes the bug on amd64.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.12+dfsg-2ubuntu5

---------------
krb5 (1.12+dfsg-2ubuntu5) trusty; urgency=low

  * Use ADD_METHOD_NOLOOP rather than ADD_METHOD for new GSS-API entry
    points, avoids infinite recursive loop when a mechanism doesn't
    provide an entry point and does include calls back into the mechglue
    (LP: #1326500)
  * Make libkadm5srv-mit8 be arch: any multi-arch: same to work around
    upgrade bug (LP: #1334052)
  * Use tailq macros to work around GCC 4.8 optimizer bug and prevent
    infinite loop for database propagation (LP: #1347147)
 -- Sam Hartman <email address hidden> Wed, 30 Jul 2014 21:06:49 -0400

Changed in krb5 (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Scott Kitterman (kitterman) wrote : Update Released

The verification of the Stable Release Update for krb5 has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello Benjamin, or anyone else affected,

Accepted gcc-4.8 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gcc-4.8/4.8.4-2ubuntu1~14.04 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in gcc-4.8 (Ubuntu Trusty):
status: Confirmed → Fix Committed
tags: removed: verification-done
tags: added: verification-needed
Revision history for this message
Matthias Klose (doko) wrote :

checked with 4.8.4-2ubuntu1~14.04

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (19.4 KiB)

This bug was fixed in the package gcc-4.8 - 4.8.4-2ubuntu1~14.04

---------------
gcc-4.8 (4.8.4-2ubuntu1~14.04) trusty-proposed; urgency=medium

  * SRU LP: #1311866.
  * Fix PR tree-optimization/63341 (wrong code, rs6000).
  * Allow to turn off -Wformat using Wno-format. LP: #1401836.
  * Fix PR target/60693 (x86, ice on valid code). LP: #1378737.
  * Fix PR tree-optimization/61964 (wrong code). LP: #1347147.
  * Fix GCC miscompilation with boost::asio::io_service::work. LP: #1338693.
  * Fix PR target/61208 (POWER, wrong code). LP: #1322287.
  * Fix ABI incompatibility between POWER and Z HTM builtins and intrinsics.
    LP: #1320292.
  * Fix PR c++/61046 (ice on invalid code). LP: #1313102.
  * Fix wrong-code issue in the little endian vector API (ppc64el).
    LP: #1311128.
  * Fix PR tree-optimization/59358 (wrong code). LP: #1395019.
  * Fix ice on ARM32. LP: #1268893.
  * Don't apply the backport for PR61841 for trusty, causing link failures.
  * Fix wrong code for vector doubleword extract (POWER). LP: #1437467.

gcc-4.8 (4.8.4-2ubuntu1) vivid; urgency=medium

  * Merge with Debian; remaining changes:
    - Build from the upstream source.

gcc-4.8 (4.8.4-2) unstable; urgency=medium

  * Update to SVN 20150426 (r222448) from the gcc-4_8-branch.
    - Fix PR libstdc++/60966, PR c/61553, PR middle-end/63704,
      PR target/61413 (ARM), PR target/64358 (RS6000), PR target/64479 (SH),
      PR target/64409 (x86), PR rtl-optimization/64037, PR c++/64487,
      PR c++/64251, PR c++/64297, PR fortran/63733, PR fortran/64244,
      PR c/64766, PR target/64882, PR rtl-optimization/61058,
      PR middle-end/43631, PR tree-optimization/64563, PR target/64513,
      PR middle-end/57748, PR middle-end/57748, PR target/64795,
      PR fortran/64528, PR fortran/56867, PR fortran/57023, PR c/57653,
      PR tree-optimization/63844 (OpenMP), PR middle-end/64199 (ice on valid),
      PR tree-optimization/64493 (ice on valid), PR tree-optimization/64495
      (wrong code), PR tree-optimization/56273 (diagnostics),
      PR tree-optimization/59124 (diagnostic), PR tree-optimization/64277
      (diagnostic), PR lto/65015, PR target/65163 (SH), PR target/64113 (ALPHA,
      link failure), PR rtl-optimization/64557, PR rtl-optimization/63475
      (ALPHA, wrong code), PR rtl-optimization/63483 (ALPHA, wrong code),
      PR target/64452 (AVR), PR target/64387 (x86, ice on valid),
      PR target/64979 (wrong code), PR target/64580 (rs6000),
      PR fortran/63744 (rejects valid), PR lto/65193 (ice on valid),
      PR tree-optimization/61634 (ice on valid), PR target/65196 (AVR),
      PR tree-optimization/63593 (ice on valid),
      PR tree-optimization/65063 (wrong code), PR target/65286 (rs6000),
      PR 65138/target (rs6000), PR target/53988 (SH), PR target/59593 (ARM),
      PR target/64453 (ARM), PR middle-end/65409 (ice on valid),
      PR tree-optimization/65388, PR fortran/65024 (ice),
      PR fortran/60898 (ice on valid), PR fortran/61138, PR libgfortran/60956,
      PR libstdc++/65279, PR libstdc++/65543, PR target/65849, PR target/65456,
      PR target/65787, PR c++/65727, PR c++/65721, PR fortran/56674,
      PR fortran/58813, PR fortran/590...

Changed in gcc-4.8 (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Benjamin, or anyone else affected,

Accepted gccgo-4.9 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gccgo-4.9/4.9.3-0ubuntu4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: removed: verification-done
tags: added: verification-needed
Revision history for this message
Steve Langasek (vorlon) wrote :

This is a bug specific to the C language, which is not applicable to the in-progress gccgo-4.9 SRU. Marking as resolved.

tags: added: verification-done
removed: verification-needed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.