From patchwork Mon Nov 6 21:27:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helge Deller X-Patchwork-Id: 10044541 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 5338B60247 for ; Mon, 6 Nov 2017 21:27:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 445F32A027 for ; Mon, 6 Nov 2017 21:27:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 38A1D2A02F; Mon, 6 Nov 2017 21:27:54 +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.4 required=2.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_HI,RCVD_IN_SORBS_SPAM 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 B9C852A027 for ; Mon, 6 Nov 2017 21:27:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751960AbdKFV1s (ORCPT ); Mon, 6 Nov 2017 16:27:48 -0500 Received: from mout.gmx.net ([212.227.17.20]:53424 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751046AbdKFV1r (ORCPT ); Mon, 6 Nov 2017 16:27:47 -0500 Received: from ls3530.fritz.box ([193.159.22.247]) by mail.gmx.com (mrgmx103 [212.227.17.168]) with ESMTPSA (Nemesis) id 0MIuft-1eEIpn0LVp-002YVw; Mon, 06 Nov 2017 22:27:38 +0100 Date: Mon, 6 Nov 2017 22:27:34 +0100 From: Helge Deller To: Christoph Biedl , Richard Henderson , John David Anglin Cc: linux-parisc@vger.kernel.org Subject: Re: Testing the lws_compare_and_swap_2 syscall Message-ID: <20171106212734.GA29237@ls3530.fritz.box> References: <1508874207@msgid.manchmal.in-ulm.de> <1508973952@msgid.manchmal.in-ulm.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1508973952@msgid.manchmal.in-ulm.de> User-Agent: Mutt/1.9.1 (2017-09-22) X-Provags-ID: V03:K0:53rnmKd2nVBHtw+EU/sAlbdweiWEC3EfzmW4OYaygkAgXvxbJnC pcQ0WTFI7FkZ2yyEWKC5bEEFSXdFhSp3dIYOEMkf7Tav7bQWjCyFV5rVT6CxBxnS9p8Qmp4 DvR6VY54DPi8eo7s2JW5mcMoQbIRvuVi2PNl2F458+eQHSZfXL3tuwGaub6OUGLzTQFDj4H jzUrJtZ3JyRsOXfYxkMzw== X-UI-Out-Filterresults: notjunk:1; V01:K0:IzpSnakbhsM=:hkmspg5R9VBLbsSWri/Ed3 E+HkvqP/4B4iwR1k1MzxnG6d4HYGuDrVziO1uxpE4Em7T/UMn+8p/ZGsi60Lsq8nxj8Sjkzfl CvzlNyQ9olDHzkynpoLwCClPEUNoklAc0DQKIhP1r59+8xZo2j4bLQNp9+6oHKwc/0crphmum Dr6Fu9psyrrmEdYhQbrKhocMs9R6sDBfcFi4VQOgS70k5OlsGkZGqefgESDOIZf+MFqlWzFWM LHlN/TCOuHjJ18EBV9n3LYAEbCZd+kIbSYDd1Aj1ehsyGkDziNjKdsHLGAjU56xV0qKEVq+2E 7VsgPfKPTcRvQ6UPXSMrdw+71nGFeu6GxToOzVA1w3YhgrV99CAyRGuv0v3wP4ewA+awlvIOa 4oB5/EYxiJ5ASe3rsGVnPmlKbuEEgDyB+KpTrLIua+8ouIkKZjjdYbYHDL76Dap4R3SpDkyvK +7eIzzVHgoJypNlfgkY2wIC///qTmtg9k1Q1Aiby9PGBpjrjRDjpx29J/gCjrM85/AukcYqpK W5o9FESkiWvk5ioZwZ8SsTb9p7lHpoRy8B21dsHJJEj1VbHjCamDTEdogrR40nVLDRjo5fVJi qXekvuE9/uXUS2DQEy/is39Y4MPaHtsCjFYwkAwqVr/CN8/SUIzQKMC6n+i0QzX/cPBEcpGJZ v1MyTF3BRGa6XLYagpBpnlfZA2WmIZg6H3OpYQfUOxOG1hcQWSiAdk54OQ3DF8Zv0ZSoreXrX mRG8KoYXGoRjHg1g7Qy9iaFHlyuYDo9dU79CDnsRSodahXOmIO2zorRYwWLXcNxqLOD1u5Sx7 WT3+iYLQM0EOUnbFAMMsraUZJB2fQ== 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 * Christoph Biedl : > 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. I'd prefer this (untested) patch: > 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? I think a CVE is not needed for parisc. There are no real productive users (I assume). > 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. Did you continued your tests? How were the results. > Aside, there is another ",ma" modifier in lws_compare_and_swap that I > fail to understand. Haven't checked yet in detail yet, though. I'd suggest to keep the other ",ma" modifiers for now. Helge --- 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 diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S index 41e60a9c7db2..f8a8754322ae 100644 --- a/arch/parisc/kernel/syscall.S +++ b/arch/parisc/kernel/syscall.S @@ -698,8 +698,7 @@ lws_compare_and_swap_2: #endif /* Check the validity of the size pointer */ - subi,>>= 4, %r23, %r0 - b,n lws_exit_nosys + cmpib,COND(<<),n 3, %r23, lws_exit_nosys /* Jump to the functions which will load the old and new values into registers depending on the their size */