Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752087AbdHZBQX (ORCPT ); Fri, 25 Aug 2017 21:16:23 -0400 Received: from mail-pg0-f42.google.com ([74.125.83.42]:37964 "EHLO mail-pg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751407AbdHZBQV (ORCPT ); Fri, 25 Aug 2017 21:16:21 -0400 Date: Fri, 25 Aug 2017 18:16:16 -0700 From: Alexei Starovoitov To: =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov , Andy Lutomirski , Arnaldo Carvalho de Melo , Casey Schaufler , Daniel Borkmann , David Drysdale , "David S . Miller" , "Eric W . Biederman" , James Morris , Jann Horn , Jonathan Corbet , Matthew Garrett , Michael Kerrisk , Kees Cook , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Shuah Khan , Tejun Heo , Thomas Graf , Will Drewry , kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org, linux-security-module@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH net-next v7 05/10] landlock: Add LSM hooks related to filesystem Message-ID: <20170826011614.iqya5dqii3n7dtdb@ast-mbp> References: <20170821000933.13024-1-mic@digikod.net> <20170821000933.13024-6-mic@digikod.net> <20170824025030.sxl2hkpcbzipb47y@ast-mbp> <22d09137-7212-5803-af64-0964fad875c7@digikod.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <22d09137-7212-5803-af64-0964fad875c7@digikod.net> User-Agent: NeoMutt/20170421 (1.8.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2907 Lines: 72 On Fri, Aug 25, 2017 at 10:16:39AM +0200, Micka?l Sala?n wrote: > > > >> +/* WRAP_ARG_SB */ > >> +#define WRAP_ARG_SB_TYPE WRAP_TYPE_FS > >> +#define WRAP_ARG_SB_DEC(arg) \ > >> + EXPAND_C(WRAP_TYPE_FS) wrap_##arg = \ > >> + { .type = BPF_HANDLE_FS_TYPE_DENTRY, .dentry = arg->s_root }; > >> +#define WRAP_ARG_SB_VAL(arg) ((uintptr_t)&wrap_##arg) > >> +#define WRAP_ARG_SB_OK(arg) (arg && arg->s_root) > > ... > > > >> +HOOK_NEW_FS(sb_remount, 2, > >> + struct super_block *, sb, > >> + void *, data, > >> + WRAP_ARG_SB, sb, > >> + WRAP_ARG_RAW, LANDLOCK_ACTION_FS_WRITE > >> +); > > > > this looks wrong. casting super_block to dentry? > > This is called when remounting a block device. The WRAP_ARG_SB take the > sb->s_root as a dentry, it is not a cast. What do you expect from this hook? got it. I missed -> part. Now it makes sense. > > > >> +/* a directory inode contains only one dentry */ > >> +HOOK_NEW_FS(inode_create, 3, > >> + struct inode *, dir, > >> + struct dentry *, dentry, > >> + umode_t, mode, > >> + WRAP_ARG_INODE, dir, > >> + WRAP_ARG_RAW, LANDLOCK_ACTION_FS_WRITE > >> +); > > > > more general question: why you're not wrapping all useful > > arguments? Like in the above dentry can be acted upon > > by the landlock rule and it's readily available... > > The context used for the FS event must have the exact same types for all > calls. This event is meant to be generic but we can add more specific > ones if needed, like I do with FS_IOCTL. I see. So all FS events will have dentry as first argument regardless of how it is in LSM hook ? I guess that will simplify the rules indeed. I suspect you're doing it to simplify the LSM->landlock shim layer as well, right? > The idea is to enable people to write simple rules, while being able to > write fine grain rules for special cases (e.g. IOCTL) if needed. > > > > > The limitation of only 2 args looks odd. > > Is it a hard limitation ? how hard to extend? > > It's not a hard limit at all. Actually, the FS_FNCTL event should have > three arguments (I'll add them in the next series): FS handle, FCNTL > command and FCNTL argument. I made sure that it's really easy to add > more arguments to the context of an event. The reason I'm asking, because I'm not completely convinced that adding another argument to existing event will be backwards compatible. It looks like you're expecting only two args for all FS events, right? How can you add 3rd argument? All FS events would have to get it, but in some LSM hooks such argument will be meaningless, whereas in other places it will carry useful info that rule can operate on. Would that mean that we'll have FS_3 event type and only few LSM hooks will be converted to it. That works, but then we'll lose compatiblity with old rules written for FS event and that given hook. Otherwise we'd need to have fancy logic to accept old FS event into FS_3 LSM hook.