| Message ID | 6889f4f9-4ae0-8a92-a2fc-04151ad8ed9f@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL] Smack: Restore the smackfsdef mount option | expand |
Casey Schaufler <casey@schaufler-ca.com> wrote: > James, this is a repair for a regression introduced in 5.1. > It should be pulled for 5.2 and added to 5.1. > > The following changes since commit 619ae03e922b65a1a5d4269ceae1e9e13a058d6b: > > Smack: Fix kbuild reported build error (2019-04-30 14:13:32 -0700) > > are available in the git repository at: > > https://github.com/cschaufler/next-smack.git smack-for-5.2-b > > for you to fetch changes up to a5765ce797070d046dc53ccceeb0ed304cb918eb: > > Smack: Restore the smackfsdef mount option (2019-05-28 10:22:04 -0700) Can you hold this for the moment, please? Note that there appears to be another problem by inspection of the code. I think that smack_sb_eat_lsm_opts() strips the "smack" prefix off of the options, whereas smack_fs_context_parse_param() does not. This means that there's no need to do this: static const struct fs_parameter_spec smack_param_specs[] = { + fsparam_string("fsdef", Opt_fsdefault), fsparam_string("fsdefault", Opt_fsdefault), fsparam_string("fsfloor", Opt_fsfloor), fsparam_string("fshat", Opt_fshat), but that all the option names in that table *do* need prefixing with "smack". The way you enter the LSM is going to depend on whether generic_parse_monolithic() is called. You're only going to enter this way if mount(2) is the syscall of entry and the filesystem doesn't override the ->parse_monolithic() option (none in the upstream kernel). David
On 5/28/2019 12:02 PM, David Howells wrote: > Casey Schaufler <casey@schaufler-ca.com> wrote: > >> James, this is a repair for a regression introduced in 5.1. >> It should be pulled for 5.2 and added to 5.1. >> >> The following changes since commit 619ae03e922b65a1a5d4269ceae1e9e13a058d6b: >> >> Smack: Fix kbuild reported build error (2019-04-30 14:13:32 -0700) >> >> are available in the git repository at: >> >> https://github.com/cschaufler/next-smack.git smack-for-5.2-b >> >> for you to fetch changes up to a5765ce797070d046dc53ccceeb0ed304cb918eb: >> >> Smack: Restore the smackfsdef mount option (2019-05-28 10:22:04 -0700) > Can you hold this for the moment, please? OK ... > Note that there appears to be another problem by inspection of the code. I > think that smack_sb_eat_lsm_opts() strips the "smack" prefix off of the > options, whereas smack_fs_context_parse_param() does not. > > This means that there's no need to do this: > > static const struct fs_parameter_spec smack_param_specs[] = { > + fsparam_string("fsdef", Opt_fsdefault), > fsparam_string("fsdefault", Opt_fsdefault), > fsparam_string("fsfloor", Opt_fsfloor), > fsparam_string("fshat", Opt_fshat), > > but that all the option names in that table *do* need prefixing with "smack". I'm not sure I follow the logic, because "mount -o smackfsdefault=Pop" does what I would expect it to. > The way you enter the LSM is going to depend on whether > generic_parse_monolithic() is called. You're only going to enter this way if > mount(2) is the syscall of entry and the filesystem doesn't override the > ->parse_monolithic() option (none in the upstream kernel). So you're saying that the code works for the mount(2) case, but won't work for some other case? Are you planning a fix? Will that fix include restoration of smackfsdef? > David
Casey Schaufler <casey@schaufler-ca.com> wrote: > > static const struct fs_parameter_spec smack_param_specs[] = { > > + fsparam_string("fsdef", Opt_fsdefault), > > fsparam_string("fsdefault", Opt_fsdefault), > > fsparam_string("fsfloor", Opt_fsfloor), > > fsparam_string("fshat", Opt_fshat), > > > > but that all the option names in that table *do* need prefixing with > > "smack". Actually, you're right, we do need to add that *and* prefix it with "smack". > I'm not sure I follow the logic, because "mount -o smackfsdefault=Pop" > does what I would expect it to. Yes, I'm sure it does - for the cases you're testing - but it's filesystem and syscall dependent. None of the filesystems currently ported to the mount API upstream override ->parse_monolithic(), but that changes with nfs, shmem and coda and will change with cifs too. It also changes if you use fsconfig() to supply the options because that goes through a different LSM hook (it uses fs_context_parse_param rather than sb_eat_lsm_opts). > > The way you enter the LSM is going to depend on whether > > generic_parse_monolithic() is called. You're only going to enter this way > > if mount(2) is the syscall of entry and the filesystem doesn't override > > the ->parse_monolithic() option (none in the upstream kernel). > > So you're saying that the code works for the mount(2) case, > but won't work for some other case? Are you planning a fix? > Will that fix include restoration of smackfsdef? I can do a fix, but testing it is a pain. David
On 5/28/2019 1:18 PM, David Howells wrote: > Casey Schaufler <casey@schaufler-ca.com> wrote: > >>> static const struct fs_parameter_spec smack_param_specs[] = { >>> + fsparam_string("fsdef", Opt_fsdefault), >>> fsparam_string("fsdefault", Opt_fsdefault), >>> fsparam_string("fsfloor", Opt_fsfloor), >>> fsparam_string("fshat", Opt_fshat), >>> >>> but that all the option names in that table *do* need prefixing with >>> "smack". > Actually, you're right, we do need to add that *and* prefix it with "smack". > >> I'm not sure I follow the logic, because "mount -o smackfsdefault=Pop" >> does what I would expect it to. > Yes, I'm sure it does - for the cases you're testing - but it's filesystem and > syscall dependent. None of the filesystems currently ported to the mount API > upstream override ->parse_monolithic(), but that changes with nfs, shmem and > coda and will change with cifs too. > > It also changes if you use fsconfig() to supply the options because that goes > through a different LSM hook (it uses fs_context_parse_param rather than > sb_eat_lsm_opts). > >>> The way you enter the LSM is going to depend on whether >>> generic_parse_monolithic() is called. You're only going to enter this way >>> if mount(2) is the syscall of entry and the filesystem doesn't override >>> the ->parse_monolithic() option (none in the upstream kernel). >> So you're saying that the code works for the mount(2) case, >> but won't work for some other case? Are you planning a fix? >> Will that fix include restoration of smackfsdef? > I can do a fix, but testing it is a pain. I will test a fix if you point me to it. I need it for 5.1 and 5.2. > > David
James, this is a repair for a regression introduced in 5.1. It should be pulled for 5.2 and added to 5.1. The following changes since commit 619ae03e922b65a1a5d4269ceae1e9e13a058d6b: Smack: Fix kbuild reported build error (2019-04-30 14:13:32 -0700) are available in the git repository at: https://github.com/cschaufler/next-smack.git smack-for-5.2-b for you to fetch changes up to a5765ce797070d046dc53ccceeb0ed304cb918eb: Smack: Restore the smackfsdef mount option (2019-05-28 10:22:04 -0700) ---------------------------------------------------------------- Casey Schaufler (1): Smack: Restore the smackfsdef mount option security/smack/smack_lsm.c | 2 ++ 1 file changed, 2 insertions(+)