From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f74.google.com (mail-wr1-f74.google.com [209.85.221.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7C19E3016EE for ; Tue, 9 Dec 2025 02:25:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.74 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765247123; cv=none; b=QxaVExLaPjYd+hdOSUGKAmNFPe3TWbH+6M0smcfE+FwSkJF/tGqNfBIXpJTLauqWB771HOmxbmqL3LdMIXiHOb5QHDjTBmblb96M+IhiobnCe8qSkXFp/fc7hBfkm6ySCdiLb8jsQsVGCsx+acykD/pny7Hli9GngTbV+bJnsxM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765247123; c=relaxed/simple; bh=gfr+WSGQUeE3tRbER1Q7ncCEzXJheoWP9nJUSk8PaZI=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=B2cHXXd7iZhOCnIhe3ZkHa1TrxvBlXP+sVMT7t9MaNPYN0/qZlBCS0GFUCFdTTbJGf9XYXGD9TO2sfmE3q/herMiLOMZKyaOrx2H1G+5+yktcK0hRdRIcyA97w/HK9beqa0z4/OpTbQ94cc1CRVfK0rzgXpD2Ex1JQgNcx6J2wI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--jackmanb.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=N+lkqSsY; arc=none smtp.client-ip=209.85.221.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--jackmanb.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="N+lkqSsY" Received: by mail-wr1-f74.google.com with SMTP id ffacd0b85a97d-42e2d02b528so3494912f8f.0 for ; Mon, 08 Dec 2025 18:25:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1765247118; x=1765851918; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=fhuWLS5fTL+DTVMTjxhbeF8m7ga3oboOLspu3QuJmuA=; b=N+lkqSsYpfK8gJrKo09ANqFnYzS3mPxYCvWgwM76HHFbmNCLo1xZdrTzFrX8ch7lTp cMLyQ9DqUccdM8ee6mbU4ilONitS7WqlOrIfKGZ9rrmeZwlz4JuyHB33g7cMDAHS15lB lZUKRRS9C/xXVyG2+F6z+SwpM9uXaEtPl9HvshtOYV6U0C57BNwKPh0YkSL4I3mAFSa8 3AwCXOKKYIjdYn9QamXGPBT5UP3TvRYRsGQ37shc9nj7mUilsUfThzw1VV2ZfoE5uzJX NMhgiNm3LLpbcUjEX0zz0rcDXcHVq7nNSa8mQMfsJD8fFP8VygVHBKLiPWv7rs+4fYER 8cJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765247118; x=1765851918; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=fhuWLS5fTL+DTVMTjxhbeF8m7ga3oboOLspu3QuJmuA=; b=A9Fqr8BjmVV9umgcqv1MTWvXVFjYrKDUbZszownhsW77XkFWU3/BuFekON3fuwkDvn lvbb+0MIe56VJWQuum/B9cnsZmBWiH8OAKsTXeBk9ifZevaZlTTqu6vEUD8jwPmUuTdM u5Wj7jQD69ChVPSTgmB1hCw5ak6a9HVw1L/siGUNYuA3K/ReZzFf5sBcIGufu2r9YItz 7Mwo6SdD56fcsW2/u/c2Qmo0WyBXh8S/X+1et2H7AyFlHH70AfinOtgz8IUkFsSt58fN 3YHuwpySfZrdEXaoLQxUzRPytTtzxrIY8r0mZ9jpBmgqNFEnfDsNyaYMM9qh5UPYMWAZ XDrg== X-Forwarded-Encrypted: i=1; AJvYcCXmHPUg9/XpmIHS44LQEPyYtviDvD1Or6PTkZ7hLM7y5EEz/nO6cEF7aQfbZbYJEDcYmSuXW0YuOgo9/00=@vger.kernel.org X-Gm-Message-State: AOJu0YzEv13MUOzt0NjkVghhP9SB0MSvXsasbRsR1ha+YYiegCjYDpBs qRB5WVW2Pjc3tXJ4PlbembxfxPvUl2yqwBjuyrfWBnbyaK4QL1P+ZCsw69WYILrwS27V/9qaAEo 0RZxRTq20gq7tGA== X-Google-Smtp-Source: AGHT+IGyIfm3muE1wjm2kHRx1oCJ6206KPRGqpQ/JAFZbzp/tGh+UNYghOA/z3ICxHyFmV3Deyf8K5aiUbxtTA== X-Received: from wruh13.prod.google.com ([2002:a5d:688d:0:b0:42b:4c63:868a]) (user=jackmanb job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6000:4028:b0:427:454:43b4 with SMTP id ffacd0b85a97d-42f89f56dcdmr9816619f8f.48.1765247118114; Mon, 08 Dec 2025 18:25:18 -0800 (PST) Date: Tue, 09 Dec 2025 02:25:17 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20251208-gcov-inline-noinstr-v1-0-623c48ca5714@google.com> X-Mailer: aerc 0.21.0 Message-ID: Subject: Re: [PATCH 0/2] Noinstr fixes for K[CA]SAN with GCOV From: Brendan Jackman To: Marco Elver , Brendan Jackman Cc: Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Ard Biesheuvel , , Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote: > On Tue, 9 Dec 2025 at 01:05, Brendan Jackman wrote: >> >> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote: >> > On Mon, 8 Dec 2025 at 10:37, Marco Elver wrote: >> >> >> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman wr= ote: >> >> > >> >> > Details: >> >> > >> >> > - =E2=9D=AF=E2=9D=AF clang --version >> >> > Debian clang version 19.1.7 (3+build5) >> >> > Target: x86_64-pc-linux-gnu >> >> > Thread model: posix >> >> > InstalledDir: /usr/lib/llvm-19/bin >> >> > >> >> > - Kernel config: >> >> > >> >> > https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e1865= 7174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt >> >> > >> >> > Note I also get this error: >> >> > >> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to = !ENDBR: machine_kexec_prepare+0x810 >> >> > >> >> > That one's a total mystery to me. I guess it's better to "fix" the = SEV >> >> > one independently rather than waiting until I know how to fix them = both. >> >> > >> >> > Note I also mentioned other similar errors in [0]. Those errors don= 't >> >> > exist in Linus' master and I didn't note down where I saw them. Eit= her >> >> > they have since been fixed, or I observed them in Google's internal >> >> > codebase where they were instroduced downstream. >> >> > >> >> > This is a successor to [1] but I haven't called it a v2 because it'= s a >> >> > totally different solution. Thanks to Ard for the guidance and >> >> > corrections. >> >> > >> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.c= om/ >> >> > >> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-5= 4f7790d54df@google.com/ >> >> >> >> Why is [1] not the right solution? >> >> The problem is we have lots of "inline" functions, and any one of the= m >> >> could cause problems in future. >> > >> > Perhaps I should qualify: lots of *small* inline functions, including >> > those stubs. >> > >> >> I don't mind turning "inline" into "__always_inline", but it seems >> >> we're playing whack-a-mole here, and just disabling GCOV entirely >> >> would make this noinstr.c file more robust. >> > >> > To elaborate: `UBSAN_SANITIZE_noinstr.o :=3D n` and >> > `K{A,C}SAN_SANITIZE_noinstr.o :=3D n` is already set on this file. >> > Perhaps adding __always_inline to the stub functions here will be >> > enough today, but might no longer be in future. >> >> Well you can also see it the other way around: disabling GCOV_PROFILE >> might be enough today, but as soon as some other noinstr disables >> __SANITIZE_ADDRESS__ and expects to be able to call instrumented >> helpers, that code will be broken too. > > This itself is a contradiction: a `noinstr` function should not call > instrumented helpers. Normally this all works due to the compiler's > function attributes working as intended for the compiler-inserted > instrumentation, but for explicitly inserted instrumentation it's > obviously not. In otherwise instrumented files with few (not all) > `noinstr` functions, making the stub functions `__always_inline` will > not work, because the preprocessor is applied globally not per > function. In the past, I recall the underlying implementation being > used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions > to solve that. Sorry I dropped an important word here, I meant to say other noinstr _files_. I.e. anything else similar to SEV's noinstr.c that is doing noinstr at the file level. >> Still, despite my long-winded arguments I'm not gonna die on this hill, >> I would be OK with both ways. > > To some extent I think doing both to reduce the chance of issues in > future might be what you want. On the other hand, avoiding the > Makefile-level opt-out will help catch more corner cases in future, > which may or may not be helpful outside this noinstr.c file. Cool, then yeah I think I will do both unless anyone shows up to object to that. Both things ultimately make sense on their own merit and even if you only need one or the other to make the error go away, I don't think that actually makes them "redundant". >> > The alternative is to audit the various sanitizer stub functions, and >> > mark all these "inline" stub functions as "__always_inline". The >> > changes made in this series are sufficient for the noinstr.c case, but >> > not complete. >> >> Oh, yeah I should have done __kcsan_{en,di}able_current() too I think. >> >> Are there other stubs you are thinking of? I think we only care about th= e >> !__SANITIZE_*__ stubs - we don't need this for !CONFIG_* stubs, right? >> Anything else I'm forgetting? > > Initially, I think !__SANITIZE_* stubs are enough. Well, basically > anything that appears in , because all those are > __always_inline, we should make the called functions also > __always_inline. Ack, thanks for all the input here! I'll probably wait until after LPC to do a v2.