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=-4.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,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 60CA6C4727C for ; Tue, 29 Sep 2020 12:17:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D5A3C20759 for ; Tue, 29 Sep 2020 12:17:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Lpx5Ohd0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730290AbgI2MQ7 (ORCPT ); Tue, 29 Sep 2020 08:16:59 -0400 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:36086 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730189AbgI2MQt (ORCPT ); Tue, 29 Sep 2020 08:16:49 -0400 Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1601381807; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=C1hoLwkXB3t3tvpnB48LHPZiaL0srQuk1qO89dBM94g=; b=Lpx5Ohd0cRpOxImPQBECg6oeq122xPk4sbHh3hZimE2YTs1KAXfR39eNCtbCGCFgMoix+4 YQWhqe3HxY8gVyvVHCNjgYqDUG4H32RLzaynAmkv2Lxb6oT5evsQYiuyn8KkebkIJAS0QA EIzRWozdqwj/KhwE6zc0a5eO5bZ5dWU= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-426-UofWkZrfOL26fOAXaIpbiw-1; Tue, 29 Sep 2020 08:16:45 -0400 X-MC-Unique: UofWkZrfOL26fOAXaIpbiw-1 Received: by mail-lj1-f197.google.com with SMTP id 184so1127315ljf.14 for ; Tue, 29 Sep 2020 05:16:45 -0700 (PDT) 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=C1hoLwkXB3t3tvpnB48LHPZiaL0srQuk1qO89dBM94g=; b=oSczSJN2jaYwMfATyHFluSEBJbk3tiaiqnTD0/S6D1QbOHVPKkekxUWwT9iK+MKPsv kHC6qz8qJwd7qJWvnVcmFYIxcN2Nb813a0+x8cIOgnpaDT+7rsH3sPI4tZI4pZ1zZRwl D4FmH3+y9AJ6mMijHNHd/yWydPzYopGA86W7lGl+kTKvoITqQ9576NzLHzqHdIZzqxiA PAgUZm1jB+3Q6MXIS+s1FbZGcFxudvJSzsD5OjjYgikENcwG4wu+tzOKOqzim9Zm4nUF xiQshZjdLVjCs4FnjOJsxh+zSnrJ3ADcpw5fuGg5VKW59Tmuu01Jx4SZByNyy7StCCiu 8dAw== X-Gm-Message-State: AOAM531uhh+6YWOdTAtLoAJqnkyMLopT2NpW4DmebCHv/yn285hLoBkX zjH8xcit+r+PgYrulZwnOGfn4BHb0QMMYkolf+D2C5nC3KmCcjFXQOcH03KsU5eFRCxQYkU1odC 4T3A7GjribSqnzHVX/paipyQh/F4pkquE8JYDAgr16wwI2nNlDZuo X-Received: by 2002:a05:651c:10d:: with SMTP id a13mr1017976ljb.217.1601381803824; Tue, 29 Sep 2020 05:16:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw0wAN1g0mJoPRemnBeRoOHrscZEmxfN7P/VaV8PCgYFEgJUUhx3QFC5sJAzFENjRdqM8X9hzSDmaCflJ7kHnM= X-Received: by 2002:a05:651c:10d:: with SMTP id a13mr1017968ljb.217.1601381803483; Tue, 29 Sep 2020 05:16:43 -0700 (PDT) MIME-Version: 1.0 References: <20200921160922.GA23870@lst.de> <20200921163011.GZ3421308@ZenIV.linux.org.uk> <0764629d33d151aee743d0429ac87a5b0c300235.camel@themaw.net> <05c18390d485ae6d84c49f707d20b49e28f210a6.camel@themaw.net> In-Reply-To: <05c18390d485ae6d84c49f707d20b49e28f210a6.camel@themaw.net> From: Ondrej Mosnacek Date: Tue, 29 Sep 2020 14:16:32 +0200 Message-ID: Subject: Re: Commit 13c164b1a186 - regression for LSMs/SELinux? To: Ian Kent Cc: Linus Torvalds , Stephen Smalley , Al Viro , Christoph Hellwig , autofs@vger.kernel.org, Linux Security Module list , SElinux list , Zdenek Pytela Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=omosnace@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: On Sun, Sep 27, 2020 at 5:08 AM Ian Kent wrote: > > On Fri, 2020-09-25 at 10:38 -0700, Linus Torvalds wrote: > > On Fri, Sep 25, 2020 at 6:38 AM Ondrej Mosnacek > > wrote: > > > On Thu, Sep 24, 2020 at 4:16 PM Stephen Smalley > > > wrote: > > > > Up-thread I thought Linus indicated he didn't really want a flag > > > > to > > > > disable pemission checking due to potential abuse (and I agree). > > > > > > IIUC he was against adding an FMODE flag, while I was rather > > > suggesting a new function parameter (I realize it probably wasn't > > > clear from what I wrote). > > > > I really would prefer neither. > > > > Any kind of dynamic behavior that depends on a flag is generally > > worse > > than something that can be statically seen. > > > > Now, if the flag is _purely_ a constant argument in every single > > user, > > and there's no complex flow through multiple different layers, an > > argument flag is certainly fairly close to just having two different > > functions for two different behaviors. > > > > But I don't really see much of an advantage to adding a new argument > > to kernel_write() for this - because absolutely *nobody* should ever > > use it apart from this very special autofs case. > > > > So I'd rather just re-export the old __kernel_write() (or whatever it > > was that broke autofs) that didn't do that particular check. We > > already use it for splice and core dumping. > > > > autofs isn't that different from those two, and I think the only real > > difference is that autofs is a module. No? > > It can be, yes, many distro builds compile it in. > > > > > So I think the fix is as simple as exporting __kernel_write() again - > > and let's just make it a GPL-only export since we really don't want > > anybody to use it - and revert commit 13c164b1a186 ("autofs: switch > > to kernel_write"). > > Yes, sorry I missed this initially. > > There are a couple of other sanity checks in kern_write() but since > __kern_write() is meant to be for internal use that's not really > an issue IMHO. OK, so it seems that reverting comes out as the best choice here. BTW, I'm looking at rw_verify_area() and I see this "If (ppos)" check and the comment above it... And then I look at autofs_write(), which passes &file->f_pos, while ksys_write() passes file_ppos(file), which checks FMODE_STREAM and returns NULL if it is set. And since the autofs pipe should be a... well... pipe, which AFAIK implies FMODE_STREAM, file_ppos() should return NULL for it. So shouldn't autofs_write() use file_ppos(file) instead of &file->f_pos? Not sure if there are any practical implications, but seems more correct/consistent that way... And in that case most of the rw_verify_area() checks would be skipped anyway. And file_start_write()/file_end_write() do nothing on non-regular files, so it seems kernel_write() vs. __kernel_write() makes only very little difference for pipes. -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.