From: David Miller <davem@davemloft.net> To: sandeen@sandeen.net Cc: matorola@gmail.com, sparclinux@vger.kernel.org, linux-xfs@vger.kernel.org Subject: Re: [sparc64] crc32c misbehave Date: Thu, 01 Jun 2017 23:33:29 -0400 (EDT) [thread overview] Message-ID: <20170601.233329.698047529665811283.davem@davemloft.net> (raw) In-Reply-To: <9902b59c-0f73-f306-28e0-fea7ee4a1169@sandeen.net> From: Eric Sandeen <sandeen@sandeen.net> Date: Thu, 1 Jun 2017 21:10:50 -0500 > On ARM, there was a gcc bug causing similar results - I /think/ > it was https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > > "programs could fail sporadically with this if an interrupt happens at > the wrong instant in time and data was written onto the current stack." > > https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02292.html > > Maybe totally unrelated; if not, hope it helps. :) Wow, that looks exactly like what the bug is: crc32c: .register %g2, #scratch save %sp, -176, %sp ! sethi %hi(tfm), %g1 !, tmp121 mov %i2, %o2 ! length, ldx [%g1+%lo(tfm)], %g2 ! tfm, tfm.0_4 mov %i1, %o1 ! address, lduw [%g2], %g1 ! tfm.0_4->descsize, tfm.0_4->descsize add %g1, 38, %g1 ! tfm.0_4->descsize,, tmp126 srlx %g1, 4, %g1 ! tmp126,, tmp127 sllx %g1, 4, %g1 ! tmp127,, tmp128 sub %sp, %g1, %sp !, tmp128, add %sp, 2230, %i5 !,, tmp130 Ok, %i5 holds the stack address of the shash context: ... return %i7+8 lduw [%o5+16], %o0 ! MEM[(u32 *)__shash_desc.1_10 + 16B], 'return' deallocates the stack frame plus the register window, and at the same time does a delayed control transfer to "%i7 + 8". So in the branch delay slot instruction %i5 becomes %o5. And here we are accessing deallocated stack memory in the delay slot. I'm using gcc-6.3.0 here. And indeed the following patch makes the problem go away: diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c index 74a54b7..bf831e2 100644 --- a/lib/libcrc32c.c +++ b/lib/libcrc32c.c @@ -43,7 +43,7 @@ static struct crypto_shash *tfm; u32 crc32c(u32 crc, const void *address, unsigned int length) { SHASH_DESC_ON_STACK(shash, tfm); - u32 *ctx = (u32 *)shash_desc_ctx(shash); + u32 ret, *ctx = (u32 *)shash_desc_ctx(shash); int err; shash->tfm = tfm; @@ -53,7 +53,9 @@ u32 crc32c(u32 crc, const void *address, unsigned int length) err = crypto_shash_update(shash, address, length); BUG_ON(err); - return *ctx; + ret = *ctx; + barrier(); + return ret; } EXPORT_SYMBOL(crc32c);
WARNING: multiple messages have this Message-ID (diff)
From: David Miller <davem@davemloft.net> To: sandeen@sandeen.net Cc: matorola@gmail.com, sparclinux@vger.kernel.org, linux-xfs@vger.kernel.org Subject: Re: [sparc64] crc32c misbehave Date: Fri, 02 Jun 2017 03:33:29 +0000 [thread overview] Message-ID: <20170601.233329.698047529665811283.davem@davemloft.net> (raw) In-Reply-To: <9902b59c-0f73-f306-28e0-fea7ee4a1169@sandeen.net> From: Eric Sandeen <sandeen@sandeen.net> Date: Thu, 1 Jun 2017 21:10:50 -0500 > On ARM, there was a gcc bug causing similar results - I /think/ > it was https://gcc.gnu.org/bugzilla/show_bug.cgi?idc293 > > "programs could fail sporadically with this if an interrupt happens at > the wrong instant in time and data was written onto the current stack." > > https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02292.html > > Maybe totally unrelated; if not, hope it helps. :) Wow, that looks exactly like what the bug is: crc32c: .register %g2, #scratch save %sp, -176, %sp ! sethi %hi(tfm), %g1 !, tmp121 mov %i2, %o2 ! length, ldx [%g1+%lo(tfm)], %g2 ! tfm, tfm.0_4 mov %i1, %o1 ! address, lduw [%g2], %g1 ! tfm.0_4->descsize, tfm.0_4->descsize add %g1, 38, %g1 ! tfm.0_4->descsize,, tmp126 srlx %g1, 4, %g1 ! tmp126,, tmp127 sllx %g1, 4, %g1 ! tmp127,, tmp128 sub %sp, %g1, %sp !, tmp128, add %sp, 2230, %i5 !,, tmp130 Ok, %i5 holds the stack address of the shash context: ... return %i7+8 lduw [%o5+16], %o0 ! MEM[(u32 *)__shash_desc.1_10 + 16B], 'return' deallocates the stack frame plus the register window, and at the same time does a delayed control transfer to "%i7 + 8". So in the branch delay slot instruction %i5 becomes %o5. And here we are accessing deallocated stack memory in the delay slot. I'm using gcc-6.3.0 here. And indeed the following patch makes the problem go away: diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c index 74a54b7..bf831e2 100644 --- a/lib/libcrc32c.c +++ b/lib/libcrc32c.c @@ -43,7 +43,7 @@ static struct crypto_shash *tfm; u32 crc32c(u32 crc, const void *address, unsigned int length) { SHASH_DESC_ON_STACK(shash, tfm); - u32 *ctx = (u32 *)shash_desc_ctx(shash); + u32 ret, *ctx = (u32 *)shash_desc_ctx(shash); int err; shash->tfm = tfm; @@ -53,7 +53,9 @@ u32 crc32c(u32 crc, const void *address, unsigned int length) err = crypto_shash_update(shash, address, length); BUG_ON(err); - return *ctx; + ret = *ctx; + barrier(); + return ret; } EXPORT_SYMBOL(crc32c);
next prev parent reply other threads:[~2017-06-02 3:33 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CADxRZqzgaL4ew6PVek3WBsdwo6GcT0ORx=7h+6p0V3NAr8qF+w@mail.gmail.com> 2017-05-31 11:56 ` [sparc64] crc32c misbehave Anatoly Pugachev 2017-05-31 11:56 ` Anatoly Pugachev 2017-05-31 12:12 ` Anatoly Pugachev 2017-05-31 12:12 ` Anatoly Pugachev 2017-05-31 15:53 ` David Miller 2017-05-31 15:53 ` David Miller 2017-05-31 16:03 ` David Miller 2017-05-31 16:03 ` David Miller 2017-05-31 16:19 ` Eric Sandeen 2017-05-31 16:19 ` Eric Sandeen 2017-05-31 16:31 ` Eric Sandeen 2017-05-31 16:31 ` Eric Sandeen 2017-05-31 16:49 ` David Miller 2017-05-31 16:49 ` David Miller 2017-06-01 21:44 ` David Miller 2017-06-01 21:44 ` David Miller 2017-06-02 1:57 ` David Miller 2017-06-02 1:57 ` David Miller 2017-06-02 2:10 ` Eric Sandeen 2017-06-02 2:10 ` Eric Sandeen 2017-06-02 3:33 ` David Miller [this message] 2017-06-02 3:33 ` David Miller 2017-06-02 3:34 ` Eric Sandeen 2017-06-02 3:34 ` Eric Sandeen 2017-06-06 19:05 ` David Miller 2017-06-06 19:05 ` David Miller 2017-06-06 19:09 ` Eric Sandeen 2017-06-06 19:09 ` Eric Sandeen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170601.233329.698047529665811283.davem@davemloft.net \ --to=davem@davemloft.net \ --cc=linux-xfs@vger.kernel.org \ --cc=matorola@gmail.com \ --cc=sandeen@sandeen.net \ --cc=sparclinux@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.