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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A125C433FE for ; Tue, 11 Oct 2022 13:01:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229786AbiJKNBq (ORCPT ); Tue, 11 Oct 2022 09:01:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229507AbiJKNBp (ORCPT ); Tue, 11 Oct 2022 09:01:45 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9F02B6610C for ; Tue, 11 Oct 2022 06:01:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665493303; 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=gYkbIZV8EzdpKCpV967WQefXGdnAKk84YbcL80Rkuk8=; b=ZEz6R2MoXz2LZ1QxlS2HXyhJf7pafDenNqo9pgA+F4NYk8eOrhiWdGmpnSlgaBCvqdBTmA +Vy6aCVEDRFmIbKsbmllR3ywjlIoxl84bVFXVROZpyv8f7dV1LJTq2co+wJPiBo66q6Vs2 2VZnba4p70c3FASO6ONI17iEF+AhIes= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-390-BDjl_kOuMXSOXKIoPioxeQ-1; Tue, 11 Oct 2022 09:01:41 -0400 X-MC-Unique: BDjl_kOuMXSOXKIoPioxeQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1BFFB1012460; Tue, 11 Oct 2022 13:01:40 +0000 (UTC) Received: from localhost (ovpn-194-111.brq.redhat.com [10.40.194.111]) by smtp.corp.redhat.com (Postfix) with ESMTP id BBFEC2166B26; Tue, 11 Oct 2022 13:01:39 +0000 (UTC) From: Petr Lautrbach To: Stephen Smalley Cc: Paul Moore , selinux@vger.kernel.org, Mike Palmiotto Subject: Re: unnecessary log output in selinux_status_updated In-Reply-To: References: <87ilkwxbde.fsf@redhat.com> <87fsfzyc31.fsf@redhat.com> Date: Tue, 11 Oct 2022 15:01:39 +0200 Message-ID: <87a662y0ng.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Stephen Smalley writes: > On Fri, Oct 7, 2022 at 9:53 AM Petr Lautrbach wrote: >> >> Stephen Smalley writes: >> >> > On Fri, Oct 7, 2022 at 4:54 AM Petr Lautrbach wrote: >> >> >> >> Hi, >> >> >> >> >> >> Commit 05bdc03130d74 ("libselinux: use kernel status page by default") changed >> >> selinux_status_updated() so that it calls avc_process_policyload() and >> >> avc_process_setenforce() and both functions call avc_log() and avc_log() logs to >> >> stderr by default. So when a process like `rpm` checks whether there was a >> >> change, it gets output on stderr which previously wasn't there. >> >> >> >> >> >> Before this change: >> >> >>> from selinux import * >> >> >>> selinux_status_open(0); >> >> 0 >> >> >>> >> >> >>> selinux_status_updated(); >> >> 0 >> >> >>> selinux_mkload_policy(0); >> >> 0 >> >> >>> selinux_status_updated(); >> >> 1 >> >> >> >> Current version: >> >> >>> from selinux import * >> >> elinux_status_updated(); >> >> selinux_mkload_policy(0); >> >> selinux_status_updated(); >> >> >>> selinux_status_open(0); >> >> 0 >> >> >>> selinux_status_updated(); >> >> 0 >> >> >>> selinux_mkload_policy(0); >> >> 0 >> >> >>> selinux_status_updated(); >> >> uavc: op=load_policy lsm=selinux seqno=2 res=11 >> >> >> >> >> >> The calling process could set its callback but it seems unnecessarily >> >> complicated just for selinux_status_updated() which is supposed to check whether >> >> something has changed or not. Also processing events in this function seems to >> >> be unnecessary. >> >> >> >> It looks like the reason for the new code added to selinux_status_updated() is >> >> that there were several avc_netlink_check_nb() calls replaced by >> >> selinux_status_updated(). Given the problem described above, I don't think it's >> >> correct and I would like to change selinux_status_updated() back and use another >> >> mechanism that would help with the replacement. >> >> >> >> >> >> So what do you think about it? >> > >> > The goal was to switch the AVC and all of its users (e.g. >> > selinux_check_access) over to using the much more efficient SELinux >> > kernel status page mechanism for setenforce and policy load >> > notifications on newer kernels instead of the SELinux netlink >> > mechanism (which imposed extra syscall overhead on the critical path). >> > >> > Understand your concern but unsure as to whether we can just change >> > selinux_status_updated() back now. >> > Would require an audit of all users of selinux_status_updated(), both >> > direct and indirect, to ensure that none of them are relying on this >> > behavior. We can obviously fix the callers within libselinux but >> > addressing external callers is more problematic and is arguably an ABI >> > change. Would need to look at all users of the AVC, >> > selinux_check_access(), etc. >> > This change happened 2 years ago so I have to wonder why it is only >> > coming up now? >> >> Nobody has noticed it. >> >> 83 avc_log(SELINUX_POLICYLOAD, >> 84 "%s: op=load_policy lsm=selinux seqno=%u res=1", >> 85 avc_prefix, seqno); >> >> There's missing '\n' and so this message is sooner or later overwritten by >> something else, see >> https://bugzilla.redhat.com/show_bug.cgi?id=2123637 and >> https://bugzilla.redhat.com/show_bug.cgi?id=2123719 > > Ok, regardless, we need to ensure that changing it won't break > systemd, dbus, or any other userspace object managers. rpm maintainer decided to implement a logging callback [1], and so there's no need to change this back. [1] https://github.com/rpm-software-management/rpm/pull/2201 Petr