From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25886 invoked by alias); 28 Jul 2018 04:49:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 25876 invoked by uid 89); 28 Jul 2018 04:49:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=crazy, yo X-HELO: mail-oi0-f65.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=HjGhq9D6T+VoSjrNPL15M/+r6uNczhiJQqJNqlyoWFY=; b=LY9jo/bY7oWfVn1SHcujhtCeYqnXOTUdp8UpKfyDpDKmZyNZ0gZwHnHgHREXbIuvDi KFJn4qI3XMWkhvqNdS+5Xt+rESnVRomkfneiJP45AAEoUEORMicA1JjSzkzgGM5CWCTG OiTQrjH/aWax9SVuTMg3+RjbksrqnRNzcT1ou6030QYGt40Qz4oL42uOBSxcdR8kioZF iPQ0+lF6j0UE7yr4mS/pgXiYKPh531T+2CyB6nBR7gCWYUieJG4XxUsFk6c7uaPNn+E9 ci2l/9QRtbSpxpVNAuLJ5xsyiiePo8dcf2bwkHfqwN1l4Uxa13fbeW/dlK3EfvBS2JOL aoKQ== MIME-Version: 1.0 From: "H.J. Lu" Date: Sat, 28 Jul 2018 04:49:00 -0000 Message-ID: Subject: [PATCH] x86/CET: Fix property note parser To: Florian Weimer Cc: GNU C Library , "Carlos O'Donell" Content-Type: multipart/mixed; boundary="0000000000009b42dd057207f567" X-SW-Source: 2018-07/txt/msg01008.txt.bz2 --0000000000009b42dd057207f567 Content-Type: text/plain; charset="UTF-8" Content-length: 3380 On Fri, Jul 27, 2018 at 1:39 PM, Florian Weimer wrote: > On 07/27/2018 08:47 PM, H.J. Lu wrote: >> >> On Fri, Jul 27, 2018 at 11:26 AM, Florian Weimer >> wrote: >>> >>> On 07/27/2018 08:22 PM, H.J. Lu wrote: >>>> >>>> >>>> - while (1) >>>> + while (ptr < ptr_end) >>>> { >>>> unsigned int type = *(unsigned int *) ptr; >>>> unsigned int datasz = *(unsigned int *) (ptr + 4); >>> >>> >>> >>> You need 1 byte, but 8 bytes. Why is checking for at least 1 byte >>> sufficient here? >>> >> >> There is: >> >> /* Check for invalid property. */ >> if (note->n_descsz < 8 >> || (note->n_descsz % sizeof (ElfW(Addr))) != 0) >> break; >> >> before that. n_descsz should be correct. > > > I do not have a strong opinion regarding this matter. For correctly > generated notes, your patch should be fine. > There is a real bug in the note parser. It doesn't check each item. This test should fail on CET SDV since IBT is on and endbr64 is missing in foo. But it passed: [hjl@gnu-cet-1 bad-property-1]$ cat y.S #include .text .globl main .type main, @function main: .cfi_startproc endbr64 subq $8, %rsp .cfi_def_cfa_offset 16 movq $foo, %rdi call *%rdi xorl %eax, %eax addq $8, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc .size main, .-main .p2align 4,,15 .type foo, @function foo: .cfi_startproc ret .cfi_endproc .size foo, .-foo #if __SIZEOF_PTRDIFF_T__ == 8 # define ALIGN 3 #elif __SIZEOF_PTRDIFF_T__ == 4 # define ALIGN 2 #endif .section ".note.gnu.property", "a" .p2align ALIGN .long 1f - 0f /* name length */ .long 5f - 2f /* data length */ .long 5 /* note type */ 0: .asciz "GNU" /* vendor name */ 1: .p2align ALIGN 2: .long 1 /* pr_type. */ .long 4f - 3f /* pr_datasz. */ 3: .long 0x800 .long 0x800 4: .p2align ALIGN 5: .section .note.GNU-stack,"",@progbits [hjl@gnu-cet-1 bad-property-1]$ make y gcc -fcf-protection -c -o y.o y.S gcc -fcf-protection -g -o y y.o [hjl@gnu-cet-1 build-x86_64-linux]$ ../../glibc-cet-O3/build-x86_64-linux/elf/ld.so --library-path . ~/bugs/libc/bad-property-1/y [hjl@gnu-cet-1 build-x86_64-linux]$ This patch fixes it by properly checking each item and stops searching when a GNU_PROPERTY_X86_FEATURE_1_AND property is found regardless if its size is correct or not. Linker won't generate bad GNU_PROPERTY_X86_FEATURE_1_AND. But people may do crazy things. Starting program: /export/build/gnu/tools-build/glibc-cet/build-x86_64-linux/elf/ld.so --library-path . ~/bugs/libc/bad-property-1/y Program received signal SIGSEGV, Segmentation fault. 0x0000000000401130 in ?? () (gdb) disass 0x0000000000401130,+30 Dump of assembler code from 0x401130 to 0x40114e: => 0x0000000000401130: retq 0x0000000000401131: nopw %cs:0x0(%rax,%rax,1) 0x000000000040113b: nopl 0x0(%rax,%rax,1) 0x0000000000401140: endbr64 0x0000000000401144: push %r15 0x0000000000401146: mov %rdx,%r15 0x0000000000401149: push %r14 0x000000000040114b: mov %rsi,%r14 End of assembler dump. (gdb) OK for master? Thanks. -- H.J. --0000000000009b42dd057207f567 Content-Type: text/x-patch; charset="US-ASCII"; name="0001-x86-CET-Fix-property-note-parser.patch" Content-Disposition: attachment; filename="0001-x86-CET-Fix-property-note-parser.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_jk4xbj580 Content-length: 2790 RnJvbSA3NTA1MmE3ZjA4YTQyNjFlYjdjNTY4ODViNTY5NzBjYTk2MzAxZDM2 IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQpGcm9tOiAiSC5KLiBMdSIgPGhq bC50b29sc0BnbWFpbC5jb20+CkRhdGU6IEZyaSwgMjcgSnVsIDIwMTggMjA6 MzQ6NTUgLTA3MDAKU3ViamVjdDogW1BBVENIXSB4ODYvQ0VUOiBGaXggcHJv cGVydHkgbm90ZSBwYXJzZXIKCkdOVV9QUk9QRVJUWV9YODZfRkVBVFVSRV8x X0FORCBtYXkgbm90IGJlIHRoZSBmaXJzdCBwcm9wZXJ0eSBpdGVtLiAgV2UK bmVlZCB0byBwcm9wZXJseSBjaGVjayBlYWNoIHByb3BlcnR5IGl0ZW0gdW50 aWwgd2UgcmVhY2ggdGhlIGVuZCBvZiB0aGUKcHJvcGVydHkgb3IgZmluZCBH TlVfUFJPUEVSVFlfWDg2X0ZFQVRVUkVfMV9BTkQuCgoJKiBzeXNkZXBzL3g4 Ni9kbC1wcm9wLmggKF9kbF9wcm9jZXNzX2NldF9wcm9wZXJ0eV9ub3RlKTog UGFyc2UKCWVhY2ggcHJvcGVydHkgaXRlbS4KLS0tCiBzeXNkZXBzL3g4Ni9k bC1wcm9wLmggfCAyMiArKysrKysrKysrKysrKy0tLS0tLS0tCiAxIGZpbGUg Y2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgOCBkZWxldGlvbnMoLSkKCmRp ZmYgLS1naXQgYS9zeXNkZXBzL3g4Ni9kbC1wcm9wLmggYi9zeXNkZXBzL3g4 Ni9kbC1wcm9wLmgKaW5kZXggMzVkM2YxNmEyMy4uOWU0ZDUxYTcxZCAxMDA2 NDQKLS0tIGEvc3lzZGVwcy94ODYvZGwtcHJvcC5oCisrKyBiL3N5c2RlcHMv eDg2L2RsLXByb3AuaApAQCAtNzMsNyArNzMsNyBAQCBfZGxfcHJvY2Vzc19j ZXRfcHJvcGVydHlfbm90ZSAoc3RydWN0IGxpbmtfbWFwICpsLAogCSAgdW5z aWduZWQgY2hhciAqcHRyID0gKHVuc2lnbmVkIGNoYXIgKikgKG5vdGUgKyAx KSArIDQ7CiAJICB1bnNpZ25lZCBjaGFyICpwdHJfZW5kID0gcHRyICsgbm90 ZS0+bl9kZXNjc3o7CiAKLQkgIHdoaWxlIChwdHIgPCBwdHJfZW5kKQorCSAg ZG8KIAkgICAgewogCSAgICAgIHVuc2lnbmVkIGludCB0eXBlID0gKih1bnNp Z25lZCBpbnQgKikgcHRyOwogCSAgICAgIHVuc2lnbmVkIGludCBkYXRhc3og PSAqKHVuc2lnbmVkIGludCAqKSAocHRyICsgNCk7CkBAIC04MiwxNyArODIs MjMgQEAgX2RsX3Byb2Nlc3NfY2V0X3Byb3BlcnR5X25vdGUgKHN0cnVjdCBs aW5rX21hcCAqbCwKIAkgICAgICBpZiAoKHB0ciArIGRhdGFzeikgPiBwdHJf ZW5kKQogCQlicmVhazsKIAotCSAgICAgIGlmICh0eXBlID09IEdOVV9QUk9Q RVJUWV9YODZfRkVBVFVSRV8xX0FORAotCQkgICYmIGRhdGFzeiA9PSA0KQor CSAgICAgIGlmICh0eXBlID09IEdOVV9QUk9QRVJUWV9YODZfRkVBVFVSRV8x X0FORCkKIAkJewotCQkgIHVuc2lnbmVkIGludCBmZWF0dXJlXzEgPSAqKHVu c2lnbmVkIGludCAqKSBwdHI7Ci0JCSAgaWYgKChmZWF0dXJlXzEgJiBHTlVf UFJPUEVSVFlfWDg2X0ZFQVRVUkVfMV9JQlQpKQotCQkgICAgbC0+bF9jZXQg fD0gbGNfaWJ0OwotCQkgIGlmICgoZmVhdHVyZV8xICYgR05VX1BST1BFUlRZ X1g4Nl9GRUFUVVJFXzFfU0hTVEspKQotCQkgICAgbC0+bF9jZXQgfD0gbGNf c2hzdGs7CisJCSAgaWYgKGRhdGFzeiA9PSA0KQorCQkgICAgeworCQkgICAg ICB1bnNpZ25lZCBpbnQgZmVhdHVyZV8xID0gKih1bnNpZ25lZCBpbnQgKikg cHRyOworCQkgICAgICBpZiAoKGZlYXR1cmVfMSAmIEdOVV9QUk9QRVJUWV9Y ODZfRkVBVFVSRV8xX0lCVCkpCisJCQlsLT5sX2NldCB8PSBsY19pYnQ7CisJ CSAgICAgIGlmICgoZmVhdHVyZV8xICYgR05VX1BST1BFUlRZX1g4Nl9GRUFU VVJFXzFfU0hTVEspKQorCQkJbC0+bF9jZXQgfD0gbGNfc2hzdGs7CisJCSAg ICB9CiAJCSAgYnJlYWs7CiAJCX0KKworCSAgICAgIC8qIENoZWNrIHRoZSBu ZXh0IHByb3BlcnR5IGl0ZW0uICAqLworCSAgICAgIHB0ciArPSBBTElHTl9V UCAoZGF0YXN6LCBzaXplb2YgKEVsZlcoQWRkcikpKTsKIAkgICAgfQorCSAg d2hpbGUgKChwdHJfZW5kIC0gcHRyKSA+PSA4KTsKIAl9CiAKICAgICAgIC8q IE5COiBOb3RlIHNlY3Rpb25zIGxpa2UgLm5vdGUuQUJJLXRhZyBhbmQgLm5v dGUuZ251LmJ1aWxkLWlkIGFyZQotLSAKMi4xNy4xCgo= --0000000000009b42dd057207f567--