From patchwork Wed Dec 14 00:11:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Noah Goldstein X-Patchwork-Id: 1715554 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=qwvxfAw4; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NWwm65fV3z23yh for ; Wed, 14 Dec 2022 11:12:22 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EB863383F085 for ; Wed, 14 Dec 2022 00:12:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EB863383F085 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1670976740; bh=vPE9XGX+PfovBZb7fya/kMStOdhJY/5adpKbHtft+/Q=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=qwvxfAw4R7YC/QtEQBlr1Ix6RlvbXqz7ZWKHxe+OMlmp+Ej3TYKWNoW01xoap9NCb 69BklobG0QDwhxz38nMTn3noas0KcdIqB302Q1yFisiOy09YzepWSRb2wuLrzF4Gds 5ENqkM0Lpy1VgiMixtnfXU2k/G9TfcZcMvu0LbKA= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 83563387223F for ; Wed, 14 Dec 2022 00:12:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 83563387223F Received: by mail-ej1-x62f.google.com with SMTP id gh17so40820532ejb.6 for ; Tue, 13 Dec 2022 16:12:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vPE9XGX+PfovBZb7fya/kMStOdhJY/5adpKbHtft+/Q=; b=xEG43Nicem/9qKiSrOm+c4zv/521kH7FLSx+vDxZXSSWKg1qLL5RtHc8g491z6z0OP 3gq/3tOme/Ife+bgqfjCqdL0RNvJk/h8a/At1AiafE7MtJQ0YmszUk+9PgU7/juhkMIY 1Q2z/h42vFms5B60Tex5di3Sffk7v0SxkKYH2fslOpOHmqOjic8qCDhaD/rfCMbVC41X IOODcuL6av1Sfdl6h2duLctzgLUp1pEDQVD/rEvmtdnE5I72hh8Ocxc9G0wniV2VpOVq Sz4SETzTryBppLe4o+hWHTsHFM3y3obxjXvqqZW7H66SqfcYCudO2N0anyKaOf8FokcD oQqA== X-Gm-Message-State: ANoB5pnWWs2gYULma4/M/9M8Ml3THfaYKBrUVGcz6GUdlbN84XfxaV/y +ffL60scOm5OtMs+NhvMDqeKtbMvzp0= X-Google-Smtp-Source: AA0mqf7cH7Iv7ARLjus4L3WcUC2ESOYWDE6guWguLKzIX31cWgX0MaCZjngZnXzvR0CoIFKUhXKOrg== X-Received: by 2002:a17:906:647:b0:7ad:d7f9:38b8 with SMTP id t7-20020a170906064700b007add7f938b8mr19049275ejb.57.1670976721910; Tue, 13 Dec 2022 16:12:01 -0800 (PST) Received: from noahgold-desk.intel.com ([192.55.55.51]) by smtp.gmail.com with ESMTPSA id b10-20020a17090636ca00b0074134543f82sm5257121ejc.90.2022.12.13.16.12.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Dec 2022 16:12:01 -0800 (PST) To: libc-alpha@sourceware.org Cc: goldstein.w.n@gmail.com, hjl.tools@gmail.com, carlos@systemhalted.org Subject: [PATCH v1] x86: Prevent SIG11 in memcmp-sse2 when data is concurrently modified [BZ #29863] Date: Tue, 13 Dec 2022 16:11:47 -0800 Message-Id: <20221214001147.2814047-1-goldstein.w.n@gmail.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Noah Goldstein via Libc-alpha From: Noah Goldstein Reply-To: Noah Goldstein Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" In the case of INCORRECT usage of `memcmp(a, b, N)` where `a` and `b` are concurrently modified as `memcmp` runs, there can be a SIG11 in `L(ret_nonzero_vec_end_0)` because the sequential logic assumes that `(rdx - 32 + rax)` is a positive 32-bit integer. To be clear, this "fix" does not mean this usage of `memcmp` is supported. `memcmp` is incorrect when the values of `a` and/or `b` are modified while its running, and that incorrectness may manifest itself as a SIG-11. That being said, if we can make the results less dramatic with no cost to regular uses cases, there is no harm in doing so. The fix replaces a 32-bit `addl %edx, %eax` with the 64-bit variant `addq %rdx, %rax`. The 1-extra byte of code size from using the 64-bit instruction doesn't contribute to overall code size as the next target is aligned and has multiple bytes of `nop` padding before it. As well all the logic between the add and `ret` still fits in the same fetch block, so the cost of this change is basically zero. The sequential logic makes the assume behind the following code: ``` /* * rsi = a * rdi = b * rdx = len - 32 */ /* cmp a[0:15] and b[0:15]. Since length is known to be [17, 32] in this case, this check is also assume to cover a[0:(31 - len)] and b[0:(31 - len)]. */ movups (%rsi), %xmm0 movups (%rdi), %xmm1 PCMPEQ %xmm0, %xmm1 pmovmskb %xmm1, %eax subl %ecx, %eax jnz L(END_NEQ) /* cmp a[len-16:len-1] and b[len-16:len-1]. */ movups 16(%rsi, %rdx), %xmm0 movups 16(%rdi, %rdx), %xmm1 PCMPEQ %xmm0, %xmm1 pmovmskb %xmm1, %eax subl %ecx, %eax jnz L(END_NEQ2) ret L(END2): /* Position first mismatch. */ bsfl %eax, %eax /* BUG IS FROM THIS. The sequential version is able to assume this value is a positive 32-bit value because first check included bytes in range a[0:(31 - len)], b[0:(31 - len)] so `eax` must be greater than `31 - len` so the minimum value of `edx` + `eax` is `(len - 32) + (32 - len) >= 0`. In the concurrent case, however, `a` or `b` could have been changed so a mismatch in `eax` less or equal than `(31 - len)` is possible (the new low bound in `(16 - len)`. This can result in a negative 32-bit signed integer, which when non-sign extended to 64-bits is a random large value out of bounds. */ addl %edx, %eax /* Crash here because 32-bit negative number in `eax` non-sign extends to out of bounds 64-bit offset. */ movzbl 16(%rdi, %rax), %ecx movzbl 16(%rsi, %rax), %eax ``` This fix is quite simple, just make the `addl %edx, %eax` 64 bit (i.e `addq %rdx, %rax`). This prevent the 32-bit non-sign extension and since `eax` still a low bound of `16 - len` the `rdx + rax` is bound by `(len - 32) - (16 - len) >= -16`. Since we have a fixed offset of `16` in the memory access this must be inbounds. --- sysdeps/x86_64/multiarch/memcmp-sse2.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S index afd450d020..34e60e567d 100644 --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S @@ -308,7 +308,7 @@ L(ret_nonzero_vec_end_0): setg %dl leal -1(%rdx, %rdx), %eax # else - addl %edx, %eax + addq %rdx, %rax movzbl (VEC_SIZE * -1 + SIZE_OFFSET)(%rsi, %rax), %ecx movzbl (VEC_SIZE * -1 + SIZE_OFFSET)(%rdi, %rax), %eax subl %ecx, %eax