From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id BBB0438386AF for ; Wed, 14 Dec 2022 03:25:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BBB0438386AF Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x52b.google.com with SMTP id m19so20730920edj.8 for ; Tue, 13 Dec 2022 19:25:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=kZFPzFwAhTFHpdyomQw16+npYlgjA6aAberiwMfMdAc=; b=m9/6G16zS/6oSbevUWpCT9f4KEVj17rEwOPMPViP/MXFSRJTbmpJk8Nptuuve+P6SD QVr4qP9mKCKKyVDlWS+fpU921ckqD9xvg/y3kPXn1c4cA/bvtwL9Jv3FeZ2RyaJk8P5v Z3e89NvxZ56zzdMvZf/QDiTKdjvdrYOKEcvFmn39z9Wqumfw+4/3iok0+T27vZwbAx56 afysBusHb9Xz6+P9/x/MpNap0tr4/pajRGHqwiAw9knwvrw4SNa/pl9Zzo42jQqPjVwj s6nL9k87mJ0m7thuXgM+3sCZGisjkgGQHJAhQioWj03op02GDBjGWBDpzdNX7M7SIkJL 7L7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kZFPzFwAhTFHpdyomQw16+npYlgjA6aAberiwMfMdAc=; b=At4HL8IZqU/QFbK3s4dM8dlP1LcQ4p96SMiEv40Na+sV/K01zrRAfdkqPPg8TK9mnZ hQN4JkrLS6/zVObAB0sZ156xgPdP7xuJBbfxLMB2gfz4WBbjVZUvf3Z8C/ZDUi3MCy3/ MhORonId8F9UvV+XOvUY2ndFOUTsQgDxI9S4mGu6kDL2yVbj7VjP7fwuhpyB2F2TTfKv mkrtBWm0iW3/dPYaG3sOYLR708n+uOh2u5HrU2vHOPLxi+vbgv6Q2wU4yneqXgpAh49N byOi7Gmg1QlFxgaxKF1Bx0adY0Iy0dXY1YAfAx/e3I7O0SPuBotyEIYvoNcmJJuN248d rtrA== X-Gm-Message-State: ANoB5pm8u5XylnWUdZytaOQ+h0h6xEdyGAnmem7z+zxVe2RuceLEffrc DsFUDlK3sIx2G7BGYLgcI46zidv8yOY= X-Google-Smtp-Source: AA0mqf4mQsLWDjuOHi24VA5NwSmiLKf2vrsf/NDyW/flMuAx4JDtOBsNqmgBZjeu0pCLjkjU91NOgA== X-Received: by 2002:a05:6402:241e:b0:45c:834b:eb59 with SMTP id t30-20020a056402241e00b0045c834beb59mr31102163eda.36.1670988346808; Tue, 13 Dec 2022 19:25:46 -0800 (PST) Received: from noahgold-desk.intel.com ([192.55.55.51]) by smtp.gmail.com with ESMTPSA id u26-20020a05640207da00b0046bc2f432dasm5666943edy.22.2022.12.13.19.25.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Dec 2022 19:25:46 -0800 (PST) From: Noah Goldstein To: libc-alpha@sourceware.org Cc: goldstein.w.n@gmail.com, hjl.tools@gmail.com, carlos@systemhalted.org Subject: [PATCH v2] x86: Prevent SIGSEGV in memcmp-sse2 when data is concurrently modified [BZ #29863] Date: Tue, 13 Dec 2022 19:25:28 -0800 Message-Id: <20221214032528.2859009-1-goldstein.w.n@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221214001147.2814047-1-goldstein.w.n@gmail.com> References: <20221214001147.2814047-1-goldstein.w.n@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 List-Id: 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 SIGSEGV 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 SIGSEGV. That being said, if we can make the results less dramatic with no cost to regular use 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 relevant sequential logic can be seen in the following pseudo-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 assumed 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 /* The sequential version is able to assume this value is a positive 32-bit value because the first check included bytes in range a[0:(31 - len)] and 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 is `(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 this is 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 prevents the 32-bit non-sign extension and since `eax` is 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 in bounds. --- 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 -- 2.34.1