From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B98E3C35FFC for ; Sat, 22 Mar 2025 03:39:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C753C280002; Fri, 21 Mar 2025 23:39:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C246D280001; Fri, 21 Mar 2025 23:39:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AECAB280002; Fri, 21 Mar 2025 23:39:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 950A6280001 for ; Fri, 21 Mar 2025 23:39:00 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 304EB1CAD5E for ; Sat, 22 Mar 2025 03:39:01 +0000 (UTC) X-FDA: 83247780882.15.636DAD9 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by imf05.hostedemail.com (Postfix) with ESMTP id 2471C100005 for ; Sat, 22 Mar 2025 03:38:58 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=sOPkPV5G; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of jannh@google.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742614739; a=rsa-sha256; cv=none; b=aD8fL6bFikX3xJARePXchD2syMW2GZBnipRHmTD4NKsuwxSawgRWvDTaIwhvuuuXeVxYzR oRHpjb0inXBZy1N6PmDDhnsS8hTpXidwNZhG9F5dj7gd5+G7KB6VcSt5ndjZkd7dAUG3LQ lZ3WdFSlinjuQeJ+YHd1VJpxsyYZrws= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=sOPkPV5G; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of jannh@google.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742614739; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=taeWF4tOpC/dpbZzcrLZz4r9LKpMvXEAscHhqUcCxXg=; b=7V34d4l7hSjsAz/hkhsq4dyVpKvaWKeyUgmPvj7iBgF7kn3V5KuRFmPOQUiafQna///URW 2Ai2+EJCABiinDXSXVGg8QTTfez4/j702HcHtZe5oQUyDmky9oiCN+od1774sakkFvVOwX 9AiwRD02a8YLbCbKd3Ifge03qooRYa4= Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-5e789411187so2203a12.1 for ; Fri, 21 Mar 2025 20:38:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1742614737; x=1743219537; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=taeWF4tOpC/dpbZzcrLZz4r9LKpMvXEAscHhqUcCxXg=; b=sOPkPV5GaaKPIHVJXif2UbSl2CVtW/N/+br7wMnlAg/jhNnUWDkiNMbnjpO8E6MaW3 GDIcydAsWbShun3T1DCY78dPutyh5PfzQoHqI29CpgfqtWGJmxtCXAUpzJPZksg/LNxe 9FPErIzHuj/8gjGaQU7WjnGV9HyO9I7eyzJI09o/cF5zakp/AQyxEzriomPJ2n8llpu/ tnjOPjTJ3rkRutR6TyCw2Hh+4l+VtG83X5N4Q/bkO9TAWj6W5sQOW8SGfLlN37CMgfmQ A/HZtTkZLLQG76yqJBK45/3pecAwUFJFk0vzgOX8nwo3Pp5Nu/iw/dtg0bwvPx7Q80Qe /OZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742614737; x=1743219537; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=taeWF4tOpC/dpbZzcrLZz4r9LKpMvXEAscHhqUcCxXg=; b=h9YgJxqBuHN3ww3ofGkWjmEF/WVXGDOh9JceJBKYuh7n//uUey3AIdIuHBi9lLJSuy qCjrLck3CZ6r/+TgddhmOISW1rmrAfvMXU2c1D6rQcW2nHpNZAQAHp9xq4XiP1IZP8c3 mmDT1hNnZrO5b4YkvyfcVmORnyMuwU9uxnOAB4Mmm/w8aPprD2j8oe8VYhLxiSyida8m uMPA2Xi6Va64o4mPD2GIWSehiCMC/zomoa/5K5rQ4wSblQFpTIlal58x1jdtA4/01Zcu dyQcPNL6pSsGynOe1DQvkWVNVGneO8ptKl/19XksjGEHE/m2PJuIbhi/XItakrcK2QRK Rhgg== X-Forwarded-Encrypted: i=1; AJvYcCVcqDHtBzTeqpi23OEMd294rBBo+9YT1MEWm1/duZnArOqVi5KGuOp372hk1YSAJnZQ45aLN6wlhQ==@kvack.org X-Gm-Message-State: AOJu0YyqP1piPAJfuWqDrdUiA6mxx3YmiBXu0RGgvCf6ZxDx/U92JrnO bwgV5v2RSbcyr844OKq/SgIbIeekhG1tn2AG/MFIzwfMFIvhmo+zV/Y2vKls+/CviHIS7Fr5CDz nlT3u7i/cHnntLcFweAoLE/P0LNigk/mu++8oRrGrtXKQtSWx8w== X-Gm-Gg: ASbGncsyshxnksPsX7/1CWJ5I0Y0T0It2GSDfvJpy/amt42iuoNeFxDqaDmh6dO7k3h SRMNafBxnYTUiySiW09aOk9bqv4pYPDmG19cxIZ/4KGU4NwArPn1PxXmm9bnV6NGJt1J4aPdjwB YUbc3EayDHQHEpwl6bzUCDWFimexxqwsLc6hXzwgaoaOssRMO6TbBMvC4oEisuSRpS X-Google-Smtp-Source: AGHT+IFNrGflVA5cVLtkj7CWn/rMxuADVf09QwjGTQAYNADzVFRbZd+RM0Y15L/a3KXLWPWdZKicKZiySK9z7Q+N71o= X-Received: by 2002:aa7:d806:0:b0:5e4:afad:9a83 with SMTP id 4fb4d7f45d1cf-5ec1d9c7e3bmr51324a12.2.1742614737221; Fri, 21 Mar 2025 20:38:57 -0700 (PDT) MIME-Version: 1.0 References: <20250321202620.work.175-kees@kernel.org> <20250321204105.1898507-3-kees@kernel.org> In-Reply-To: <20250321204105.1898507-3-kees@kernel.org> From: Jann Horn Date: Sat, 22 Mar 2025 04:38:21 +0100 X-Gm-Features: AQ5f1Jpi8VffjP4kVgfgtjD_F_g43Fb3J-5CEl65oTN-4tWjDA9t4jwdPOVGgzw Message-ID: Subject: Re: [PATCH 3/5] compiler_types: Introduce __is_lvalue() To: Kees Cook Cc: Vlastimil Babka , Miguel Ojeda , Nathan Chancellor , Marco Elver , Nick Desaulniers , Przemek Kitszel , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-hardening@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 2471C100005 X-Stat-Signature: px1c8uca96mr6arfikhigotytgqqsosx X-Rspam-User: X-Rspamd-Server: rspam06 X-HE-Tag: 1742614738-260670 X-HE-Meta: U2FsdGVkX18VsSni+bkrVKIrrqpejHXL4Zlv7AXbfF1LsGp0SGfFJMeVxCp+JhucQk1Pelx3kpc8js2fw3BCiX31yS9m1qQlv0dAk84IArjDGdI3COYxArwuAdn7gANmTcgDO/VcVXz/Fjw0RAD0uwy+GHLY3tfC3jR6v00pbX8k6129HWP5uYJfsbrln7ufTsagFkAEA370F/tDIw7McVkQwVp+mgYk9JOTt28j4K6Nk/Y+NXibmJyrwbEdE4NKYdvaSzVsYNV6bIQdoUvBPm6WUjLPpzCuSUjcXIoRrkMZVNIZEsWtA1eXizLfPvL0bV4ii0lyBlV+A44ep1Wc5f0swwJBLrKQ4+M04adsr06cno+Gy+bJkHihD7jktGDxJOuqR1lw96iOkNqqiSsFHbSW99NKOD6VOT99HB7MKBkZ2vzm27jhYaqvw4no95Mf4rBXKLJpM9l9m8sHWvF9g5VHlBNH9M3KX83Abi80N6Zm6qXvV3fxsE2VZrAmWjrneegqgNM0+V6uX78adk3iOytePBMOzcAofXdEgu9uQsA7uMpKwEYldF4aMOOG/LqlkXnFihyv9MTtIq/6qa7jqEtDIMM4+rJElzLV6U2XKYIwwiTzUleiOYOc2Sw1okR2CO8f1ory2Fn3y14xXmJbGzhKu8SMWu84g7M1V+s3csMbUqmRBy76EVC5Yx9ILO4Y/jx8yUIrmQd59/v9xoD3PqGFPC2gfujq+SJiL4mp0WI2hzEHMzV4hqukPc1u6+lyN7vgOhQ03hWvbn6LgM/fLWNs6O9icYXeFQMpbqjPZDgJPadyNgfQAaQwOsrvKqwQlNAHyhJUBV2t59g6ZidBSCBUgQw/6tfhUznZWKGg61ekkspPJX+XhXXVF6OhCdcb1PqQJjDCOjTiv4xE7ZfCaYQ0A7eAtWKxN32H7QuGddqFMEB/eEN9UiV2Zbnpx+/7/lZ+toZA8p5RTu4YQiD 89x9M1DP HdrzL/qwBMFUzcvUQxu8DRgs5nrSsYRT86jeOGA3o6o+a0t0RqeiAeMBX3Q8KWNtfyjlxQa4k24cLvd1xbZR0PuhxbZYAH9P+Y/hgBe+L0FsLga+vqsgkLunbU0ZYfnzp+PD83UMgCvewPygJ4YEctH+fMD5E53pInU5fDMcs6eXjjlQj22+JXdpZ/G8wVXw2Vxt94dKNkkzmT+9AIE4nZvRZcQqu9GJoLBlQfw37oPj+YTXunOxe+rVA3T5rIa8M2oOpf/3RrzMvBiZtx4RqO8LP5kMoCiTQvivSQ9rSaq96gA9+sszWoL5p7tgFU/PdVT37f8ApP2FqjnZn6DE3zZ0qvGmauda2jqDg1Oj7K4HqIUk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.048244, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Mar 21, 2025 at 9:41=E2=80=AFPM Kees Cook wrote: > If __builtin_is_lvalue() is available, use it with __is_lvalue(). There > is patch to Clang to provide this builtin now[1]. > > Link: https://github.com/kees/llvm-project/commits/builtin_is_lvalue/ [1] > Signed-off-by: Kees Cook Before you land that LLVM patch, it might be a good idea to figure out how this interacts with the fun C quirk where you can have temporary rvalues which can contain array members to which you can technically create lvalues but must not write. As far as I understand, calling "kfree(getS().ptrs[0])" as in the following example would cause UB with your patch: ``` $ cat kees-kfree-test.c #include #define __is_lvalue(expr) __builtin_is_lvalue(expr) void __kfree(void *ptr); void BEFORE_SET_TO_NULL(); void AFTER_SET_TO_NULL(); static inline void kfree_and_null(void **ptr) { __kfree(*ptr); BEFORE_SET_TO_NULL(); *ptr =3D NULL; AFTER_SET_TO_NULL(); } #define __force_lvalue_expr(x) \ __builtin_choose_expr(__is_lvalue(x), x, (void *){ NULL }) #define __free_and_null(__how, x) \ ({ \ typeof(x) *__ptr =3D &(x); \ __how ## _and_null((void **)__ptr); \ }) #define __free_and_maybe_null(__how, x) \ __builtin_choose_expr(__is_lvalue(x), \ __free_and_null(__how, __force_lvalue_expr(x)), \ __kfree(x)) #define kfree(x) __free_and_maybe_null(kfree, x) struct S { void *ptrs[1]; }; struct S getS(void); int is_lvalue_test(void) { return __is_lvalue(getS().ptrs[0]); } void testfn2(void) { kfree(getS().ptrs[0]); } $ [...]/bin/clang-14 -c -o kees-kfree-test.o kees-kfree-test.c -O3 -Wall $ objdump -d -Mintel -r kees-kfree-test.o kees-kfree-test.o: file format elf64-x86-64 Disassembly of section .text: 0000000000000000 : 0: b8 01 00 00 00 mov eax,0x1 5: c3 ret 6: 66 2e 0f 1f 84 00 00 cs nop WORD PTR [rax+rax*1+0x0] d: 00 00 00 0000000000000010 : 10: 50 push rax 11: e8 00 00 00 00 call 16 12: R_X86_64_PLT32 getS-0x4 16: 48 89 c7 mov rdi,rax 19: e8 00 00 00 00 call 1e 1a: R_X86_64_PLT32 __kfree-0x4 1e: 31 c0 xor eax,eax 20: e8 00 00 00 00 call 25 21: R_X86_64_PLT32 BEFORE_SET_TO_NULL-0x4 25: 31 c0 xor eax,eax 27: 59 pop rcx 28: e9 00 00 00 00 jmp 2d 29: R_X86_64_PLT32 AFTER_SET_TO_NULL-0x4 jannh@horn:~/test/kees-kfree$ ``` As far as I understand, this causes UB in C99 ("If an attempt is made to modify the result of a function call or to access it after the next sequence point, the behavior is undefined.") and in C11 ("A non-lvalue expression with structure or union type, where the structure or union contains a member with array type (including, recursively, members of all contained structures and unions) refers to an object with automatic storage duration and temporary lifetime. 36) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends when the evaluation of the containing full expression or full declarator ends. Any attempt to modify an object with temporary lifetime results in undefined behavior."). Basically, something like getS().ptrs[0] gives you something that is technically an lvalue but must not actually be written to, and ->isModifiableLvalue() does not catch that.