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=-3.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 CFAD6C433DB for ; Wed, 20 Jan 2021 08:02:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7732823158 for ; Wed, 20 Jan 2021 08:02:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730235AbhATIC0 (ORCPT ); Wed, 20 Jan 2021 03:02:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53964 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730243AbhATH7p (ORCPT ); Wed, 20 Jan 2021 02:59:45 -0500 Received: from mail-vk1-xa29.google.com (mail-vk1-xa29.google.com [IPv6:2607:f8b0:4864:20::a29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C14CC0613C1 for ; Tue, 19 Jan 2021 23:59:05 -0800 (PST) Received: by mail-vk1-xa29.google.com with SMTP id v3so5465574vkb.1 for ; Tue, 19 Jan 2021 23:59:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=szeredi.hu; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=raG85U5m6BVpkFn9tnlYSwbmBuD+wqMz8bt05EHQnpE=; b=COBq+q3DyHX/wjdBbdPiY06GVMPcsbaUsOQZTyJ7qWCOT9Qwlsa14PlxON1EmmP3dn ZyLBJZlg9mscFGi0pxRYe1PkgnxzZ7cc4PGSDkCygYvQXkYEPB+iqlRQKVsFYRij4TSP 0se7PqcCswNtOkSxGRxAGUJHRKavvDfakuICQ= 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=raG85U5m6BVpkFn9tnlYSwbmBuD+wqMz8bt05EHQnpE=; b=Nsx5lKA5j0hc/nCMpJZg7H3JvvqiMxtfAdG9a6ktJdb7LeU1y3vIuzbx0+HsTgeHyS 0Hd6ts+QEnHgevSAgGeXp4Q2DIPDyyl5IKEK/DXOODag6qPYopSTwwoh8NPluO9ve+tU 4F6F/fKN3mOQqBzGkWysZOqlaFFfzIsheLi0duNqjcF9gEYTf5H/uUrORHMiCnDH86jA +j11IBa9MCBKhj1BTpilvhYgNfL8DgWnYSZYP8Lze0VbsBt/6WSxFJ0cIEOQLG9BJKNn ztT5wSuEdWbqccGBZpEKTP+epfZ87cx9v/X6OwqUT/kRTFFDMJGNX9KowpKX6ySsZkEp Ot2w== X-Gm-Message-State: AOAM530Qg8PUcIEUENdTV+O/0ZXniNEIDy4MNvt7+I6dFGGcHAGuEEuc zTsjFu0koxhFiiFy0aJ0RpKGLVyfbS8ZpiV2aMVDcw== X-Google-Smtp-Source: ABdhPJxTrW09RNX/58RsaWlPlSBaSO57A7eNZ40/KxN4vYPs/WQG40kBkXd8dat0tMmqg4Q+Tp0YtsfcCmv0wTVCO0Q= X-Received: by 2002:a1f:410c:: with SMTP id o12mr5782747vka.19.1611129544253; Tue, 19 Jan 2021 23:59:04 -0800 (PST) MIME-Version: 1.0 References: <20210119162204.2081137-1-mszeredi@redhat.com> <20210119162204.2081137-3-mszeredi@redhat.com> <8735yw8k7a.fsf@x220.int.ebiederm.org> In-Reply-To: <8735yw8k7a.fsf@x220.int.ebiederm.org> From: Miklos Szeredi Date: Wed, 20 Jan 2021 08:58:53 +0100 Message-ID: Subject: Re: [PATCH 2/2] security.capability: fix conversions on getxattr To: "Eric W. Biederman" Cc: Miklos Szeredi , linux-fsdevel@vger.kernel.org, overlayfs , LSM , linux-kernel@vger.kernel.org, "Serge E . Hallyn" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 20, 2021 at 2:39 AM Eric W. Biederman wrote: > > Miklos Szeredi writes: > > > If a capability is stored on disk in v2 format cap_inode_getsecurity() will > > currently return in v2 format unconditionally. > > > > This is wrong: v2 cap should be equivalent to a v3 cap with zero rootid, > > and so the same conversions performed on it. > > > > If the rootid cannot be mapped v3 is returned unconverted. Fix this so > > that both v2 and v3 return -EOVERFLOW if the rootid (or the owner of the fs > > user namespace in case of v2) cannot be mapped in the current user > > namespace. > > This looks like a good cleanup. > > I do wonder how well this works with stacking. In particular > ovl_xattr_set appears to call vfs_getxattr without overriding the creds. > What the purpose of that is I haven't quite figured out. It looks like > it is just a probe to see if an xattr is present so maybe it is ok. Yeah, it's checking in the removexattr case whether copy-up is needed or not (i.e. if trying to remove a non-existent xattr, then no need to copy up). But for consistency it should also be wrapped in override creds. Adding fix to this series. I'll also audit for any remaining omissions. One known and documented case is vfs_ioctl(FS_IOC_{[SG]ETFLAGS,FS[SG]ETXATTR}), but that shouldn't be affected by user namespaces. Thanks, Miklos