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 X-Spam-Level: X-Spam-Status: No, score=-13.2 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F717C43461 for ; Thu, 29 Apr 2021 18:47:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D7AF56143E for ; Thu, 29 Apr 2021 18:47:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241057AbhD2Srs (ORCPT ); Thu, 29 Apr 2021 14:47:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41138 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233565AbhD2Srs (ORCPT ); Thu, 29 Apr 2021 14:47:48 -0400 Received: from mail-oo1-xc2a.google.com (mail-oo1-xc2a.google.com [IPv6:2607:f8b0:4864:20::c2a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 46EA5C06138B for ; Thu, 29 Apr 2021 11:47:01 -0700 (PDT) Received: by mail-oo1-xc2a.google.com with SMTP id k9-20020a4ad1090000b02901f9e46845e7so540999oor.6 for ; Thu, 29 Apr 2021 11:47:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8+UJwrU2MphE2H9pVBe4Va+3pVl8MrgEBW+di5Ar9x8=; b=YheD8PJ4vjevdlZN9xjnFDpXx4r8xj2WfocSQQ4FGrmpUl25K4XoAHracDIHAGonKN Jv3mQKHDRRNGM9uEqJMtY4Yp9sJ3Z6THXD+9dAm2ae5aXwuyvkAQ/SuhvQyXFZNln9i2 akSju72aiY34YA1tJymgIHYtICD8WkUWRdIjCAjzxQr8A9Poah+p2KGQoWPwUYfMK1g7 aLIoo8lvbq17uIsuedHL2iNKfjB94cT0cHbrYZbZxc/kkTs6hyLzneEYpH3ZydvnAXZ0 jvxrcdJ+BRzf2j1qvZOSHDGDCPF+loGh+oihiIW+0yzNjgqWWgkE7+/VoNUsYlxK75OI yKow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8+UJwrU2MphE2H9pVBe4Va+3pVl8MrgEBW+di5Ar9x8=; b=lyC8FthjUBK7GqoymbxJsKiMC9seSyScl9jSl9F6Ef5LRLtpAfxMcMg89UBAq5OEO9 vov8WhaHu1iz+PAbK59bx7grKxqkXelR3zQRAuzutYYLRUtYIVoTS/TtKja4ioNvD7jn oFU7r4mjTI70fkgfuGK0H46OZUAfHEqTanBo71L9nGpgCRoeUEvslUg6D0M3LO7Ravm2 XWR2Yj/SiHRbl0XsB/+6To4JOnOGmbEyOuZS54nR5phgk0+96tfeZQf1houcZo3+JSM+ WCqbSefQFgH+ffBNqhvqZejiGmbDIPuMeCBhWqqGEVLf89LvPk6iCXNPMJtGwe5tLWBv xw2A== X-Gm-Message-State: AOAM533eSPREGX60YndX/8/GQrzJdzWatLGpTPeEay5FUM+fung3c8Cc znKSqeamUahHCdYBOj52F4QjBY7XGAKTlc2xULDUzg== X-Google-Smtp-Source: ABdhPJxg+cX3WOkcKuWZN0i123++XM7omVXz3QzNsOO657nSkyD05K2ZNT5GkuRpNxSGCUdZmf57iALTgxI/591rfdA= X-Received: by 2002:a4a:96e3:: with SMTP id t32mr1153302ooi.14.1619722020332; Thu, 29 Apr 2021 11:47:00 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Marco Elver Date: Thu, 29 Apr 2021 20:46:48 +0200 Message-ID: Subject: Re: siginfo_t ABI break on sparc64 from si_addr_lsb move 3y ago To: "Eric W. Biederman" Cc: Florian Weimer , "David S. Miller" , Arnd Bergmann , Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Peter Collingbourne , Dmitry Vyukov , Alexander Potapenko , sparclinux@vger.kernel.org, linux-arch , LKML , linux-api@vger.kernel.org, kasan-dev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-api@vger.kernel.org On Thu, 29 Apr 2021 at 19:24, Eric W. Biederman wrote: [...] > > Granted, nobody seems to have noticed because I don't even know if these > > fields have use on sparc64. But I don't yet see this as justification to > > leave things as-is... > > > > The collateral damage of this, and the acute problem that I'm having is > > defining si_perf in a sort-of readable and portable way in siginfo_t > > definitions that live outside the kernel, where sparc64 does not yet > > have broken si_addr_lsb. And the same difficulty applies to the kernel > > if we want to unbreak sparc64, while not wanting to move si_perf for > > other architectures. > > > > There are 2 options I see to solve this: > > > > 1. Make things simple again. We could just revert the change moving > > si_addr_lsb into the union, and sadly accept we'll have to live with > > that legacy "design" mistake. (si_perf stays in the union, but will > > unfortunately change its offset for all architectures... this one-off > > move might be ok because it's new.) > > > > 2. Add special cases to retain si_addr_lsb in the union on architectures > > that do not have __ARCH_SI_TRAPNO (the majority). I have added a > > draft patch that would do this below (with some refactoring so that > > it remains sort-of readable), as an experiment to see how complicated > > this gets. > > > > Which option do you prefer? Are there better options? > > Personally the most important thing to have is a single definition > shared by all architectures so that we consolidate testing. > > A little piece of me cries a little whenever I see how badly we > implemented the POSIX design. As specified by POSIX the fields can be > place in siginfo such that 32bit and 64bit share a common definition. > Unfortunately we did not addpadding after si_addr on 32bit to > accommodate a 64bit si_addr. I think it's even worse than that, see the fun I had with siginfo last week: https://lkml.kernel.org/r/20210422191823.79012-1-elver@google.com ... because of the 3 initial ints and no padding after them, we can't portably add __u64 fields to siginfo, and are forever forced to have subtly different behaviour between 32-bit and 64-bit architectures. :-/ > I find it unfortunate that we are adding yet another definition that > requires translation between 32bit and 64bit, but I am glad > that at least the translation is not architecture specific. That common > definition is what has allowed this potential issue to be caught > and that makes me very happy to see. > > Let's go with Option 3. > > Confirm BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR are not > in use on any architecture that defines __ARCH_SI_TRAPNO, and then fixup > the userspace definitions of these fields. > > To the kernel I would add some BUILD_BUG_ON's to whatever the best > maintained architecture (sparc64?) that implements __ARCH_SI_TRAPNO just > to confirm we don't create future regressions by accident. > > I did a quick search and the architectures that define __ARCH_SI_TRAPNO > are sparc, mips, and alpha. All have 64bit implementations. A further > quick search shows that none of those architectures have faults that > use BUS_MCEERR_AR, BUS_MCEERR_AO, SEGV_BNDERR, SEGV_PKUERR, nor do > they appear to use mm/memory-failure.c > > So it doesn't look like we have an ABI regression to fix. That sounds fine to me -- my guess was that they're not used on these architectures, but I just couldn't make that call. I have patches adding compile-time asserts for sparc64, arm, arm64 ready to go. I'll send them after some more testing. Thanks, -- Marco