From patchwork Thu Oct 26 00:22:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Biedl X-Patchwork-Id: 10027367 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0CA486032C for ; Thu, 26 Oct 2017 00:22:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F175628C6E for ; Thu, 26 Oct 2017 00:22:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E4A0328C8F; Thu, 26 Oct 2017 00:22:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3DDC528C6E for ; Thu, 26 Oct 2017 00:22:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932197AbdJZAWX (ORCPT ); Wed, 25 Oct 2017 20:22:23 -0400 Received: from manchmal.in-ulm.de ([217.10.9.201]:44006 "EHLO manchmal.in-ulm.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932194AbdJZAWX (ORCPT ); Wed, 25 Oct 2017 20:22:23 -0400 Date: Thu, 26 Oct 2017 02:22:17 +0200 From: Christoph Biedl To: linux-parisc@vger.kernel.org Subject: Re: Testing the lws_compare_and_swap_2 syscall Message-ID: <1508973952@msgid.manchmal.in-ulm.de> References: <1508874207@msgid.manchmal.in-ulm.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP John David Anglin wrote... > On 2017-10-24, at 4:03 PM, Christoph Biedl wrote: > > > To start with, using the invalid value 4 as size parameter does not > > return ENOSYS as I'd expect but crashes my system[2], using both 32 and > > 64 bit kernel, no root privileges required. > > /* Check the validity of the size pointer */ > subi,>>= 4, %r23, %r0 > b,n lws_exit_nosys > > The condition in the subi instruction should be ">>". The branch is incorrectly nullified when > %r23 is 4. Ups, way too obvious. This does the trick, or: Tested-By: Should I try to get a CVE number for this, or is parisc considered *that* historical nobody actually cares? Now, using the given cmpxchg2 function, the following code tests this LWS for 32bit: The first test is for *mem == *old, the second for *mem != *old. Is there anything wrong with this? uint32_t mem; uint32_t old; uint32_t new; mem = 1; old = 1; new = 3; cmpxchg2 (&mem, &old, &new, 2); if (mem == old) { printf ("PASS: mem unchanged\n"); } else { printf ("FAIL: got = 0x%x, expected = 0x%x\n", mem, old); } mem = 1; old = 3; new = 3; cmpxchg2 (&mem, &old, &new, 2); if (mem == new) { printf ("PASS: mem changed\n"); } else { printf ("FAIL: got = 0x%x, expected = 0x%x\n", mem, new); } My problem here: Both tests fail, and so do more complex ones that test the other data sizes as well. After staring at lws_compare_and_swap_2 a long time it seems there are two issues: First, there is more usage of ",ma" so an update of mem/r26 hits the wrong place. After that, all results are the wrong way around. I'm frightened to tell but I fear the logic in lws_compare_and_swap_2 is inverted in each and every place. I'm happy to be convinced otherwise. But for the time being it seems the patch below is an improvement. Status: My system boots and all my tests pass. However, I find smartd stalling in 100% CPU, might be coincidence. Now I'm putting some more load onto the box to see whether it's less crashy then it used to be the previous weeks. Aside, there is another ",ma" modifier in lws_compare_and_swap that I fail to understand. Haven't checked yet in detail yet, though. Cheers, Christoph --- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/src/arch/parisc/kernel/syscall.S +++ b/src/arch/parisc/kernel/syscall.S @@ -796,30 +796,30 @@ ldo 1(%r0),%r28 /* 8bit CAS */ -13: ldb,ma 0(%r26), %r29 - sub,= %r29, %r25, %r0 +13: ldb 0(%r26), %r29 + sub,<> %r29, %r25, %r0 b,n cas2_end -14: stb,ma %r24, 0(%r26) +14: stb %r24, 0(%r26) b cas2_end copy %r0, %r28 nop nop /* 16bit CAS */ -15: ldh,ma 0(%r26), %r29 - sub,= %r29, %r25, %r0 +15: ldh 0(%r26), %r29 + sub,<> %r29, %r25, %r0 b,n cas2_end -16: sth,ma %r24, 0(%r26) +16: sth %r24, 0(%r26) b cas2_end copy %r0, %r28 nop nop /* 32bit CAS */ -17: ldw,ma 0(%r26), %r29 - sub,= %r29, %r25, %r0 +17: ldw 0(%r26), %r29 + sub,<> %r29, %r25, %r0 b,n cas2_end -18: stw,ma %r24, 0(%r26) +18: stw %r24, 0(%r26) b cas2_end copy %r0, %r28 nop @@ -827,21 +827,22 @@ /* 64bit CAS */ #ifdef CONFIG_64BIT -19: ldd,ma 0(%r26), %r29 - sub,*= %r29, %r25, %r0 +19: ldd 0(%r26), %r29 + sub,<> %r29, %r25, %r0 b,n cas2_end -20: std,ma %r24, 0(%r26) +20: std %r24, 0(%r26) copy %r0, %r28 #else /* Compare first word */ 19: ldw 0(%r26), %r29 sub,= %r29, %r22, %r0 - b,n cas2_end + b,n cas2_64set /* Compare second word */ 20: ldw 4(%r26), %r29 - sub,= %r29, %r23, %r0 + sub,<> %r29, %r23, %r0 b,n cas2_end /* Perform the store */ +cas2_64set: 21: fstdx %fr4, 0(%r26) copy %r0, %r28 #endif