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=-0.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 CCF26C433DF for ; Thu, 28 May 2020 19:10:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A9F952075A for ; Thu, 28 May 2020 19:10:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1590693026; bh=dAS4DXwHNeuhzpvWEgC5d100yXjW+kGcXfbxsRlTvG4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=DSmvcq5YxKBFJEvOMzMIp6fHjQYRt0phmHA0QXERjlc6Sy0Z12h31e2/238Vhv3Ej /UvlOulezjlTQX3jm/7Pt9OM3CMox+UWpQAmgilEQEebChb65GzWJKIR2BKJ8EWj1J bH8VSNvo+22tLy0D5pM30SpNSL6MPzf3DlM6Eks8= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406221AbgE1TK0 (ORCPT ); Thu, 28 May 2020 15:10:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57660 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406096AbgE1TKW (ORCPT ); Thu, 28 May 2020 15:10:22 -0400 Received: from mail-ej1-x641.google.com (mail-ej1-x641.google.com [IPv6:2a00:1450:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0DF95C08C5C6 for ; Thu, 28 May 2020 12:10:22 -0700 (PDT) Received: by mail-ej1-x641.google.com with SMTP id f7so857511ejq.6 for ; Thu, 28 May 2020 12:10:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xmGmOOl5gLGli7OvUQS4McOy7NB6xPaIApbCKtCeXYk=; b=f8/b3x4fYWIUj58vxbwcT1zj5CSOCsV5h++6plCNXoHkOlz5+HIWFBcJy1hj9Iocmy Q6KvNGUaf7Ktx8T3H3MKSb0yiSTTXPOtIV4i3w5okfxXhRQn9jfimGy2D5TlehUTq6Ly 3gZ8euFDfy2U4nl63gvxc9icwAuOlc7jkm8dM= 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=xmGmOOl5gLGli7OvUQS4McOy7NB6xPaIApbCKtCeXYk=; b=WQVzgg+j/z+VXFHUgpu3S5QrWttkYT7XAvoTQmtZPdwP2FSm5B5/J9azyFsaoJVvRe e9wbWIdgoMhhztOv5o4mOaIOkaqM0My7INUng0+DGFjeLbbnLckCDFIsynYjA9GPJxxN 2HUqyxhvedpPkwRHkRva5q222D+zCeUgMFYerHPyNrEza6+OVbti1kNYbCNQolKvsePR Vi7MSGRDaux5TysLziR3kXLRNj+NzpZrbi3QLyp/p7vIZXZLdp3zOJ7GyF3ZoMb/hxs7 3EIbWuXryy3RAXSczmwzdlf4mHCKhLLT4K8Y8hG2jMElPYgMJ41ovFkgA8SFwQYjozSc 9Zhg== X-Gm-Message-State: AOAM530eJKbizmHcq0L8K/tSmQ+XhsBqYmtAKHsgPJTnIIxP8augj6vn XHqUroJOcI1FVKym7dkrSP4fh4hGkd4= X-Google-Smtp-Source: ABdhPJynhJZhvhH5QoDSQ/+1tCm0BEYdLBHKSGdvphUrVfnh7T9sQdPU90FSaDrdmqn9j/9a3nmUSQ== X-Received: by 2002:a17:906:c7d1:: with SMTP id dc17mr4822181ejb.166.1590693020264; Thu, 28 May 2020 12:10:20 -0700 (PDT) Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com. [209.85.208.42]) by smtp.gmail.com with ESMTPSA id b16sm5242438edu.89.2020.05.28.12.10.19 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 May 2020 12:10:20 -0700 (PDT) Received: by mail-ed1-f42.google.com with SMTP id d24so935356eds.11 for ; Thu, 28 May 2020 12:10:19 -0700 (PDT) X-Received: by 2002:ac2:5a4c:: with SMTP id r12mr2405859lfn.10.1590692668401; Thu, 28 May 2020 12:04:28 -0700 (PDT) MIME-Version: 1.0 References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> <87eer4ysm5.fsf_-_@x220.int.ebiederm.org> In-Reply-To: <87eer4ysm5.fsf_-_@x220.int.ebiederm.org> From: Linus Torvalds Date: Thu, 28 May 2020 12:04:12 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 01/11] exec: Reduce bprm->per_clear to a single bit To: "Eric W. Biederman" Cc: Linux Kernel Mailing List , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , linux-fsdevel , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , LSM List , James Morris , "Serge E. Hallyn" , Andy Lutomirski Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 28, 2020 at 8:45 AM Eric W. Biederman wrote: > > - me->personality &= ~bprm->per_clear; > + if (bprm->per_clear) > + me->personality &= ~PER_CLEAR_ON_SETID;\ My only problem with this patch is that I find that 'per_clear' thing to be a horrid horrid name, Obviously the name didn't change, but the use *did* change, and as such the name got worse. It used do do things like bprm->per_clear |= PER_CLEAR_ON_SETID; and now it does bprm->per_clear = 1; and honestly, there's a lot more semantic context in the old code that is now missing entirely. At least you used to be able to grep for PER_CLEAR_ON_SETID and it would make you go "Ahh.." Put another way, I can kind of see what a line like bprm->per_clear |= PER_CLEAR_ON_SETID; does, simply because now it kind of hints at what is up. But what the heck does bprm->per_clear = 1; mean? Nothing. You have to really know the code. "per_clear" makes no sense, and now it's a short line that doesn't need to be that short. I think "bprm->clear_personality_bits" would maybe describe what the _effect_ of that field is. It doesn't explain _why_, but it at least explains "what" much better than "per_clear", which just makes me go "per what?". Alternatively, "bprm->creds_changed" would describe what the bit conceptually is about, and code like if (bprm->creds_changed) me->personality &= ~PER_CLEAR_ON_SETID;\ looks sensible to me and kind of matches the comment about the PER_CLEAR_ON_SETID bits are. So I think that using a bitfield is fine, but I'd really like it to be named something much better. Plus changing the name means that you can't have any code that now mistakenly uses the new semantics but expects the old bitmask. Generally when something changes semantics that radically, you want to make sure the type changes sufficiently that any out-of-tree patch that hasn't been merged yet will get a clear warning or error if people don't realize. Please? Linus