From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AB8JxZpb5hlIlbVGWHtJau77WvIVPVv+qJlyphBTwJjRcgF2RCWLEzut11NLWiQHwfztPQYdPISE ARC-Seal: i=1; a=rsa-sha256; t=1526663624; cv=none; d=google.com; s=arc-20160816; b=AB//IdEwsunkF034Swr05y1V+LYPZ5IowkuW0+Q7Y9DnZiYWsFZoqzJgnozUUsm/SK jRFPyUfi9oN+SbHL+lcsdlpVMqgzxiPE+dlynPHpmt8PATMrx8j26jre817x4FkZgqcJ 6a1uzZC8e81tDxSDd/BFbPqY9ouvSv4557qNmjaFt+zmkSdxXqGHhx/KqHJ39bZ/JPrT ii+YUjTUOY+/LyDpdPzUYCvQTDL8W8N73bKefeIOLn+yObcV8Gcn7zTADp67ZXyMUqLc /G6USjO9seo51wb4Fio4z2nu51f3/8EigYd6VOn+B7RoQkGkbePmtNXHfXXw2dKAUq86 dzlQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:arc-authentication-results; bh=psqXTlNCYMUuvDI7knMIJnx6ZWBd9uMawcn4lPh9d2Y=; b=xiIApK91vL7xDMZUqD8w3BA1GKia+z85j4njEqeXLC3CRTyJ7nLkoJY7PnymNcyDzc bf7pgKHyWG+D2FUMBhCQ9k/TWezKKBwUs0LBg3sLzyrTEIF+3fXUHXXQovTNs/hw+Dd5 L1d/UjAa5mz/odkbKBx1toOhYqqpJRZlM+XMZaQZkzOy71oORVwse1ACf98SwRgwkYep vTWfZ8vDc/6hC/2pax6DTGdW5oACCmVP0KwK8qTcj0wSbNrdXDDmEe3PeAPHW4wQ9bhQ DWgdySC1Uyb+1sDGPtcGFC8t+dUj4Ji1jyvx1RbeJx2Q3JmByM7NwN3Ev4xM1g2HoYJp X3kg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of jmorris@namei.org designates 65.99.196.166 as permitted sender) smtp.mailfrom=jmorris@namei.org Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of jmorris@namei.org designates 65.99.196.166 as permitted sender) smtp.mailfrom=jmorris@namei.org Date: Sat, 19 May 2018 03:13:37 +1000 (AEST) From: James Morris To: "Eric W. Biederman" cc: Casey Schaufler , Mimi Zohar , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, David Howells , "Luis R . Rodriguez" , kexec@lists.infradead.org, Andres Rodriguez , Greg Kroah-Hartman , Ard Biesheuvel , Kees Cook Subject: Re: [PATCH v2 3/9] security: define security_kernel_read_blob() wrapper In-Reply-To: <87y3ghhbws.fsf@xmission.com> Message-ID: References: <1526568530-9144-1-git-send-email-zohar@linux.vnet.ibm.com> <1526568530-9144-4-git-send-email-zohar@linux.vnet.ibm.com> <74c096ca-1ad1-799e-df3d-7b1b099333a7@schaufler-ca.com> <87y3ghhbws.fsf@xmission.com> User-Agent: Alpine 2.21 (LRH 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1600723179870285580?= X-GMAIL-MSGID: =?utf-8?q?1600822836684940941?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Thu, 17 May 2018, Eric W. Biederman wrote: > Nacked-by: "Eric W. Biederman" > > Nack on this sharing nonsense. These two interfaces do not share any > code in their implementations other than the if statement to distinguish > between the two cases. Hmm, it's not even doing that. There's already an if(!file && read_id == X) { } check and this is another one being added. > If we want comprehensible and maintainable code in the security modules > we need to split these two pieces of functionality apart. All ima_read is doing in both the old and new case is checking if there's no file then if it's a certain operation, returning an error. To echo Eric and Casey's suggestions, how about changing the name of the hook to security_kernel_read_data() ? Then ima_read_file() can be changed to ima_read_data(), and then instead of two if (!file && read_id == X) checks, have: if (!file) { switch (read_id) { } } -- James Morris From mboxrd@z Thu Jan 1 00:00:00 1970 From: jmorris@namei.org (James Morris) Date: Sat, 19 May 2018 03:13:37 +1000 (AEST) Subject: [PATCH v2 3/9] security: define security_kernel_read_blob() wrapper In-Reply-To: <87y3ghhbws.fsf@xmission.com> References: <1526568530-9144-1-git-send-email-zohar@linux.vnet.ibm.com> <1526568530-9144-4-git-send-email-zohar@linux.vnet.ibm.com> <74c096ca-1ad1-799e-df3d-7b1b099333a7@schaufler-ca.com> <87y3ghhbws.fsf@xmission.com> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Thu, 17 May 2018, Eric W. Biederman wrote: > Nacked-by: "Eric W. Biederman" > > Nack on this sharing nonsense. These two interfaces do not share any > code in their implementations other than the if statement to distinguish > between the two cases. Hmm, it's not even doing that. There's already an if(!file && read_id == X) { } check and this is another one being added. > If we want comprehensible and maintainable code in the security modules > we need to split these two pieces of functionality apart. All ima_read is doing in both the old and new case is checking if there's no file then if it's a certain operation, returning an error. To echo Eric and Casey's suggestions, how about changing the name of the hook to security_kernel_read_data() ? Then ima_read_file() can be changed to ima_read_data(), and then instead of two if (!file && read_id == X) checks, have: if (!file) { switch (read_id) { } } -- James Morris -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from namei.org ([65.99.196.166]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fJix3-00058R-4e for kexec@lists.infradead.org; Fri, 18 May 2018 17:13:58 +0000 Date: Sat, 19 May 2018 03:13:37 +1000 (AEST) From: James Morris Subject: Re: [PATCH v2 3/9] security: define security_kernel_read_blob() wrapper In-Reply-To: <87y3ghhbws.fsf@xmission.com> Message-ID: References: <1526568530-9144-1-git-send-email-zohar@linux.vnet.ibm.com> <1526568530-9144-4-git-send-email-zohar@linux.vnet.ibm.com> <74c096ca-1ad1-799e-df3d-7b1b099333a7@schaufler-ca.com> <87y3ghhbws.fsf@xmission.com> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: "Eric W. Biederman" Cc: Kees Cook , Ard Biesheuvel , Greg Kroah-Hartman , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, David Howells , linux-security-module@vger.kernel.org, "Luis R . Rodriguez" , Casey Schaufler , linux-integrity@vger.kernel.org, Mimi Zohar , Andres Rodriguez On Thu, 17 May 2018, Eric W. Biederman wrote: > Nacked-by: "Eric W. Biederman" > > Nack on this sharing nonsense. These two interfaces do not share any > code in their implementations other than the if statement to distinguish > between the two cases. Hmm, it's not even doing that. There's already an if(!file && read_id == X) { } check and this is another one being added. > If we want comprehensible and maintainable code in the security modules > we need to split these two pieces of functionality apart. All ima_read is doing in both the old and new case is checking if there's no file then if it's a certain operation, returning an error. To echo Eric and Casey's suggestions, how about changing the name of the hook to security_kernel_read_data() ? Then ima_read_file() can be changed to ima_read_data(), and then instead of two if (!file && read_id == X) checks, have: if (!file) { switch (read_id) { } } -- James Morris _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec