* Re: [PATCH] Smack: Restore the smackfsdef mount option
From: Casey Schaufler @ 2019-05-28 20:24 UTC (permalink / raw)
To: David Howells
Cc: LKML, Al Viro, jose.bollo, Linux Security Module list, casey
In-Reply-To: <26777166-5e0d-adfd-e59f-bcee7f18841a@schaufler-ca.com>
On 5/28/2019 12:57 PM, Casey Schaufler wrote:
> On 5/28/2019 11:54 AM, David Howells wrote:
>> Casey Schaufler <casey@schaufler-ca.com> wrote:
>>
>>>> Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>
>>>>>> Also, should all of these be prefixed with "smack"? So:
>>>>>>
>>>>>> fsparam_string("smackfsdef", Opt_fsdefault),
>>>>>> fsparam_string("smackfsfloor", Opt_fsfloor),
>>>>>> fsparam_string("smackfshat", Opt_fshat),
>>>>> No. smack_fs_parameters takes care of that.
>>>> It does? *Blink*.
>>> Well, something does. I can't say that I 100% understand all
>>> of how the new mount code handles the mount options. Y'all made
>>> sweeping changes, and the code works the way it used to except
>>> for the awkward change from smackfsdef to smackfsdefault. It
>>> took no small amount of head scratching and experimentation to
>>> convince myself that the fix I proposed was correct.
>> Ah... I suspect the issue is that smack_sb_eat_lsm_opts() strips the prefix
>> for an unconverted filesystem, but smack_fs_context_parse_param() doesn't
>> (which it shouldn't).
>>
>> Can you try grabbing my mount-api-viro branch from:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
>>
>> and testing setting smack options on a tmpfs filesystem?
> My fedora system won't boot because smackfsdef isn't recognized. :(
> I will put in my fix and retry.
No joy there, either. Now it accepts smackfsdef, but doesn't
recognize smackfsroot. I don't have this problem with vanilla
5.1.
>
>> You might need to try modifying samples/vfs/test-fsmount.c to make it mount a
>> trmpfs filesystem through the new mount UAPI.
>>
>> David
^ permalink raw reply
* Re: SGX vs LSM (Re: [PATCH v20 00/28] Intel SGX1 support)
From: Sean Christopherson @ 2019-05-28 20:24 UTC (permalink / raw)
To: Xing, Cedric
Cc: Andy Lutomirski, Stephen Smalley, Jarkko Sakkinen, James Morris,
Serge E. Hallyn, LSM List, Paul Moore, Eric Paris,
selinux@vger.kernel.org, Jethro Beekman, Hansen, Dave,
Thomas Gleixner, Dr. Greg, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F654E9824@ORSMSX116.amr.corp.intel.com>
On Sat, May 25, 2019 at 11:09:38PM -0700, Xing, Cedric wrote:
> > From: Andy Lutomirski [mailto:luto@kernel.org]
> > Sent: Saturday, May 25, 2019 5:58 PM
> >
> > On Sat, May 25, 2019 at 3:40 PM Xing, Cedric <cedric.xing@intel.com> wrote:
> > >
> > > If we think of EADD as a way of mmap()'ing an enclave file into memory,
> > > would this
> > security_enclave_load() be the same as
> > security_mmap_file(source_vma->vm_file, maxperm, MAP_PRIVATE), except that
> > the target is now EPC instead of regular pages?
> >
> > Hmm, that's clever. Although it seems plausible that an LSM would want to
> > allow RX or RWX of a given file page but only in the context of an approved
> > enclave, so I think it should still be its own hook.
>
> What do you mean by "in the context of an approved enclave"? EPC pages are
> *inaccessible* to any software until after EINIT. So it would never be a
> security concern to EADD a page with wrong permissions as long as the enclave
> would be denied eventually by LSM at EINIT.
>
> But I acknowledge the difference between loading a page into regular memory
> vs. into EPC. So it's beneficial to have a separate hook, which if not
> hooked, would pass through to security_mmap_file() by default?
Mapping the enclave will still go through security_mmap_file(), the extra
security_enclave_load() hook allows the mmap() to use PROT_NONE.
> > If it's going to be in an arbitrary file, then I think the signature needs to be more like:
> >
> > int security_enclave_init(struct vm_area_struct *sigstruct_vma, loff_t sigstruct_offset,
> > const sgx_sigstruct *sigstruct);
> >
> > So that the LSM still has the opportunity to base its decision on the contents of the
> > SIGSTRUCT. Actually, we need that change regardless.
>
> Wouldn't the pair of { sigstruct_vma, sigstruct_offset } be the same as just
> a pointer, because the VMA could be looked up using the pointer and the
> offset would then be (pointer - vma->vm_start)?
VMA has vm_file, e.g. the .sigstruct file labeled by LSMs. That being
said, why does the LSM need the VMA? E.g. why not this?
int security_enclave_init(struct file *file, struct sgx_sigstruct *sigstruct);
> > > Loosely speaking, an enclave (including initial contents of all of its pages and their
> > permissions) and its MRENCLAVE are a 1-to-1 correspondence (given the collision resistant
> > property of SHA-2). So only one is needed for a decision, and either one would lead to the
> > same decision. So I don't see anything making any sense here.
> > >
> > > Theoretically speaking, if LSM can make a decision at EINIT by means of
> > security_enclave_load(), then security_enclave_load() is never needed.
> > >
> > > In practice, I support keeping both because security_enclave_load() can only approve an
> > enumerable set while security_enclave_load() can approve a non-enumerable set of enclaves.
> > Moreover, in order to determine the validity of a MRENCLAVE (as in development of a policy
> > or in creation of a white/black list), system admins will need the audit log produced by
> > security_enclave_load().
> >
> > I'm confused. Things like MRSIGNER aren't known until the SIGSTRUCT shows
> > up. Also, security_enclave_load() provides no protection against loading a
> > mishmash of two different enclave files. I see security_enclave_init() as
> > "verify this SIGSTRUCT against your policy on who may sign enclaves and/or
> > grant EXECMOD depending on SIGSTRUCT" and security_enclave_load() as
> > "implement your EXECMOD / EXECUTE / WRITE / whatever policy and possibly
> > check enclave files for some label."
>
> Sorry for the confusion. I was saying the same thing except that the decision
> of security_enclave_load() doesn't have to depend on SIGSTRUCT. Given your
> prototype of security_enclave_load(), I think we are on the same page. I made
> the above comment to object to the idea of "require that the sigstruct be
> supplied before any EADD operations so that the maxperm decisions can depend
> on the sigstruct".
Except that having the sigstruct allows using the sigstruct as the proxy
for the enclave. I think the last big disconnect is that Andy and I want
to tie everything to an enclave-specific file, i.e. sigstruct, while you
are proposing labeling /dev/sgx/enclave. If someone wants to cram several
sigstructs into a single file, so be it, but using /dev/sgx/enclave means
users can't do per-enclave permissions, period.
What is your objection to working on the sigstruct?
> > > > > Passing both would allow tying EXECMOD to /dev/sgx/enclave as
> > > > > Cedric wanted (without having to play games and pass
> > > > > /dev/sgx/enclave to security_enclave_load()), but I don't think
> > > > > there's anything fundamentally broken with using .sigstruct for
> > > > > EXECMOD. It requires more verbose labeling, but that's not a bad thing.
> > > >
> > > > The benefit of putting it on .sigstruct is that it can be per-enclave.
> > > >
> > > > As I understand it from Fedora packaging, the way this works on
> > > > distros is generally that a package will include some files and
> > > > their associated labels, and, if the package needs EXECMOD, then the
> > > > files are labeled with EXECMOD and the author of the relevant code might get a dirty
> > look.
> > > >
> > > > This could translate to the author of an exclave that needs RWX
> > > > regions getting a dirty look without leaking this permission into other enclaves.
> > > >
> > > > (In my opinion, the dirty looks are actually the best security
> > > > benefit of the entire concept of LSMs making RWX difficult. A
> > > > sufficiently creative attacker can almost always bypass W^X
> > > > restrictions once they’ve pwned you, but W^X makes it harder to pwn
> > > > you in the first place, and SELinux makes it really obvious when
> > > > packaging a program that doesn’t respect W^X. The upshot is that a
> > > > lot of programs got fixed.)
> > >
> > > I'm lost here. Dynamically linked enclaves, if running on SGX2, would need RW->RX, i.e.
> > FILE__EXECMOD on /dev/sgx/enclave. But they never need RWX, i.e. PROCESS__EXECMEM.
> >
> > Hmm. If we want to make this distinction, we need something a big richer
> > than my proposed callbacks. A check of the actual mprotect() / mmap()
> > permissions would also be needed. Specifically, allowing MAXPERM=RWX
> > wouldn't imply that PROT_WRITE | PROT_EXEC is allowed.
Actually, I think we do have everything we need from an LSM perspective.
LSMs just need to understand that sgx_enclave_load() with a NULL vma
implies a transition from RW. For example, SELinux would interpret
sgx_enclave_load(NULL, RX) as requiring FILE__EXECMOD.
As Cedric mentioned earlier, the host process doesn't necessarily know
which pages will end up RW vs RX, i.e. sgx_enclave_load(NULL, RX)
already has to be invoked at runtime, and when that happens, the kernel
can take the opportunity to change the VMAs from MAY_RW to MAY_RX.
For simplicity in the kernel and clarity in userspace, it makes sense to
require an explicit ioctl() to add the to-be-EAUG'd range. That just
leaves us wanting an ioctl() to set the post-EACCEPT{COPY} permissions.
E.g.:
ioctl(<prefix>_ADD_REGION, { NULL }) /* NULL == EAUG, MAY_RW */
mprotect(addr, size, RW);
...
EACCEPTCOPY -> EAUG /* page fault handler */
ioctl(<prefix>_ACTIVATE_REGION, { addr, size, RX}) /* MAY_RX */
mprotect(addr, size, RX);
...
And making ACTIVATE_REGION a single-shot per page eliminates the need for
the MAXPERMS concept (see below).
> If we keep only one MAXPERM, wouldn't this be the current behavior of
> mmap()/mprotect()?
>
> To be a bit more clear, system admin sets MAXPERM upper bound in the form of
> FILE__{READ|WRITE|EXECUTE|EXECMOD} of /dev/sgx/enclave. Then for a
> process/enclave, if what it requires falls below what's allowed on
> /dev/sgx/enclave, then everything will just work. Otherwise, it fails in the
> form of -EPERM returned from mmap()/mprotect(). Please note that MAXPERM here
> applies to "runtime" permissions, while "initial" permissions are taken care
> of by security_enclave_{load|init}. "initial" permissions could be more
> permissive than "runtime" permissions, e.g., RX is still required for initial
> code pages even though system admins could disable dynamically loaded code
> pages by *not* giving FILE__{EXECUTE|EXECMOD}. Therefore, the "initial"
> mapping would still have to be done by the driver (to bypass LSM), either via
> a new ioctl or as part of IOC_EINIT.
Aha!
Starting with Cedric's assertion that initial permissions can be taken
directly from SECINFO:
- Initial permissions for *EADD* pages are explicitly handled via
sgx_enclave_load() with the exact SECINFO permissions.
- Initial permissions for *EAUG* are unconditionally RW. EACCEPTCOPY
requires the target EPC page to be RW, and EACCEPT with RO is useless.
- Runtime permissions break down as follows:
R - N/A, subset of RW (EAUG)
W - N/A, subset of RW (EAUG) and x86 paging can't do W
X - N/A, subset of RX (x86 paging can't do XO)
RW - Handled by EAUG LSM hook (uses RW unconditionally)
WX - N/A, subset of RWX (x86 paging can't do WX)
RX - Handled by ACTIVATE_REGION
RWX - Handled by ACTIVATE_REGION
In other words, if we define the SGX -> LSM calls as follows (minus the
file pointer and other params for brevity):
- <prefix>_ACTIVATE_REGION(vma, perms) -> sgx_enclave_load(NULL, perms)
- <prefix>_ADD_REGION(vma) -> sgx_enclave_load(vma, SECINFO.perms)
- <prefix>_ADD_REGION(NULL) -> sgx_enclave_load(NULL, RW)
then SGX and LSMs have all the information and hooks needed. The catch
is that the LSM semantics of sgx_enclave_load(..., RW) would need to be
different than normal shared memory, e.g. FILE__WRITE should *not* be
required, but that's ok since it's an SGX specific hook. And if for some
reason an LSM wanted to gate access to EAUG *without* FILE__EXECMOD, it'd
have the necessary information to do so.
The userspace changes are fairly minimal:
- For SGX1, use PROT_NONE for the initial mmap() and refactor ADD_PAGE
to ADD_REGION.
- For SGX2, do an explicit ADD_REGION on the ranges to be EAUG'd, and an
ACTIVATE_REGION to make a region RX or R (no extra ioctl() required to
keep RW permissions).
Because ACTIVATE_REGION can only be done once per page, to do *abitrary*
mprotect() transitions, userspace would need to set the added/activated
permissions to be a superset of the transitions, e.g. RW -> RX would
require RWX, but that's a non-issue.
- For SGX1 it's a nop since it's impossible to change the EPCM
permissions, i.e. the page would need to be RWX regardless.
- For SGX2, userspace can suck it up and request RWX to do completely
arbitrary transitions (working as intended), or the kernel can support
trimming (removing) pages from an enclave, which would allow userspace
to do "arbitrary" transitions by first removing the page.
^ permalink raw reply
* Re: [PULL] Smack: Restore the smackfsdef mount option
From: David Howells @ 2019-05-28 20:18 UTC (permalink / raw)
To: Casey Schaufler
Cc: dhowells, James Morris, Linux Security Module list, Al Viro, LKML
In-Reply-To: <6e061326-5feb-5471-c0c0-a364af5e82c3@schaufler-ca.com>
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
^ permalink raw reply
* Re: [PATCH 3/7] vfs: Add a mount-notification facility
From: Jann Horn @ 2019-05-28 20:06 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, raven, linux-fsdevel, Linux API, linux-block, keyrings,
linux-security-module, kernel list
In-Reply-To: <155905933492.7587.6968545866041839538.stgit@warthog.procyon.org.uk>
On Tue, May 28, 2019 at 6:05 PM David Howells <dhowells@redhat.com> wrote:
> Add a mount notification facility whereby notifications about changes in
> mount topology and configuration can be received. Note that this only
> covers vfsmount topology changes and not superblock events. A separate
> facility will be added for that.
[...]
> @@ -172,4 +167,18 @@ static inline void notify_mount(struct mount *changed,
> u32 info_flags)
> {
> atomic_inc(&changed->mnt_notify_counter);
> +
> +#ifdef CONFIG_MOUNT_NOTIFICATIONS
> + {
> + struct mount_notification n = {
> + .watch.type = WATCH_TYPE_MOUNT_NOTIFY,
> + .watch.subtype = subtype,
> + .watch.info = info_flags | sizeof(n),
> + .triggered_on = changed->mnt_id,
> + .changed_mount = aux ? aux->mnt_id : 0,
> + };
> +
> + post_mount_notification(changed, &n);
> + }
> +#endif
> }
[...]
> +void post_mount_notification(struct mount *changed,
> + struct mount_notification *notify)
> +{
> + const struct cred *cred = current_cred();
This current_cred() looks bogus to me. Can't mount topology changes
come from all sorts of places? For example, umount_mnt() from
umount_tree() from dissolve_on_fput() from __fput(), which could
happen pretty much anywhere depending on where the last reference gets
dropped?
> + struct path cursor;
> + struct mount *mnt;
> + unsigned seq;
> +
> + seq = 0;
> + rcu_read_lock();
> +restart:
> + cursor.mnt = &changed->mnt;
> + cursor.dentry = changed->mnt.mnt_root;
> + mnt = real_mount(cursor.mnt);
> + notify->watch.info &= ~WATCH_INFO_IN_SUBTREE;
> +
> + read_seqbegin_or_lock(&rename_lock, &seq);
> + for (;;) {
> + if (mnt->mnt_watchers &&
> + !hlist_empty(&mnt->mnt_watchers->watchers)) {
> + if (cursor.dentry->d_flags & DCACHE_MOUNT_WATCH)
> + post_watch_notification(mnt->mnt_watchers,
> + ¬ify->watch, cred,
> + (unsigned long)cursor.dentry);
> + } else {
> + cursor.dentry = mnt->mnt.mnt_root;
> + }
> + notify->watch.info |= WATCH_INFO_IN_SUBTREE;
> +
> + if (cursor.dentry == cursor.mnt->mnt_root ||
> + IS_ROOT(cursor.dentry)) {
> + struct mount *parent = READ_ONCE(mnt->mnt_parent);
> +
> + /* Escaped? */
> + if (cursor.dentry != cursor.mnt->mnt_root)
> + break;
> +
> + /* Global root? */
> + if (mnt != parent) {
> + cursor.dentry = READ_ONCE(mnt->mnt_mountpoint);
> + mnt = parent;
> + cursor.mnt = &mnt->mnt;
> + continue;
> + }
> + break;
(nit: this would look clearer if you inverted the condition and wrote
it as "if (mnt == parent) break;", then you also wouldn't need that
"continue" or the braces)
> + }
> +
> + cursor.dentry = cursor.dentry->d_parent;
> + }
> +
> + if (need_seqretry(&rename_lock, seq)) {
> + seq = 1;
> + goto restart;
> + }
> +
> + done_seqretry(&rename_lock, seq);
> + rcu_read_unlock();
> +}
[...]
> +SYSCALL_DEFINE5(mount_notify,
> + int, dfd,
> + const char __user *, filename,
> + unsigned int, at_flags,
> + int, watch_fd,
> + int, watch_id)
> +{
> + struct watch_queue *wqueue;
> + struct watch_list *wlist = NULL;
> + struct watch *watch;
> + struct mount *m;
> + struct path path;
> + int ret;
> +
> + if (watch_id < -1 || watch_id > 0xff)
> + return -EINVAL;
> +
> + ret = user_path_at(dfd, filename, at_flags, &path);
The third argument of user_path_at() contains kernel-private lookup
flags, I'm pretty sure userspace isn't supposed to be able to control
these directly.
> + if (ret)
> + return ret;
> +
> + wqueue = get_watch_queue(watch_fd);
> + if (IS_ERR(wqueue))
> + goto err_path;
> +
> + m = real_mount(path.mnt);
> +
> + if (watch_id >= 0) {
> + if (!m->mnt_watchers) {
> + wlist = kzalloc(sizeof(*wlist), GFP_KERNEL);
> + if (!wlist)
> + goto err_wqueue;
> + INIT_HLIST_HEAD(&wlist->watchers);
> + spin_lock_init(&wlist->lock);
> + wlist->release_watch = release_mount_watch;
> + }
> +
> + watch = kzalloc(sizeof(*watch), GFP_KERNEL);
> + if (!watch)
> + goto err_wlist;
> +
> + init_watch(watch, wqueue);
> + watch->id = (unsigned long)path.dentry;
> + watch->private = path.mnt;
> + watch->info_id = (u32)watch_id << 24;
> +
> + down_write(&m->mnt.mnt_sb->s_umount);
> + if (!m->mnt_watchers) {
> + m->mnt_watchers = wlist;
> + wlist = NULL;
> + }
> +
> + ret = add_watch_to_object(watch, m->mnt_watchers);
> + if (ret == 0) {
> + spin_lock(&path.dentry->d_lock);
> + path.dentry->d_flags |= DCACHE_MOUNT_WATCH;
> + spin_unlock(&path.dentry->d_lock);
> + path_get(&path);
So... the watches on a mountpoint create references back to the
mountpoint? Is your plan that umount_tree() breaks the loop by getting
rid of the watches?
If so: Is there anything that prevents installing new watches after
umount_tree()? Because I don't see anything.
It might make sense to redesign this stuff so that watches don't hold
references on the object being watched.
> + }
> + up_write(&m->mnt.mnt_sb->s_umount);
> + if (ret < 0)
> + kfree(watch);
> + } else if (m->mnt_watchers) {
> + down_write(&m->mnt.mnt_sb->s_umount);
> + ret = remove_watch_from_object(m->mnt_watchers, wqueue,
> + (unsigned long)path.dentry,
> + false);
> + up_write(&m->mnt.mnt_sb->s_umount);
> + } else {
> + ret = -EBADSLT;
> + }
> +
> +err_wlist:
> + kfree(wlist);
> +err_wqueue:
> + put_watch_queue(wqueue);
> +err_path:
> + path_put(&path);
> + return ret;
> +}
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Mimi Zohar @ 2019-05-28 20:06 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <87zhn65qor.fsf@morokweng.localdomain>
On Tue, 2019-05-28 at 16:23 -0300, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
> > Hi Thiago,
> >
> >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> >> index fca7a3f23321..a7a20a8c15c1 100644
> >> --- a/security/integrity/ima/ima_policy.c
> >> +++ b/security/integrity/ima/ima_policy.c
> >> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
> >> }
> >> }
> >>
> >> +#define __ima_hook_stringify(str) (#str),
> >> +
> >> +const char *const func_tokens[] = {
> >> + __ima_hooks(__ima_hook_stringify)
> >> +};
> >> +
> >> #ifdef CONFIG_IMA_READ_POLICY
> >> enum {
> >> mask_exec = 0, mask_write, mask_read, mask_append
> >> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
> >> "MAY_APPEND"
> >> };
> >>
> >> -#define __ima_hook_stringify(str) (#str),
> >> -
> >> -static const char *const func_tokens[] = {
> >> - __ima_hooks(__ima_hook_stringify)
> >> -};
> >> -
> >> void *ima_policy_start(struct seq_file *m, loff_t *pos)
> >> {
> >> loff_t l = *pos;
> >
> > Is moving this something left over from previous versions or there is
> > a need for this change?
>
> Well, it's not a strong need, but it's still relevant in the current
> version. I use func_tokens in ima_read_modsig() in order to be able to
> mention the hook name in mod_check_sig()'s error message:
>
> In ima_read_modsig():
>
> rc = mod_check_sig(sig, buf_len, func_tokens[func]);
>
> And in mod_check_sig():
>
> pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> name);
>
> If you think it's not worth it to expose func_tokens, I can make
> ima_read_modsig() pass a more generic const string such as "IMA modsig"
> for example.
This is fine. I somehow missed moving func_tokens[] outside of the
ifdef was in order to make it independent of "CONFIG_IMA_READ_POLICY".
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH] Smack: Restore the smackfsdef mount option
From: Casey Schaufler @ 2019-05-28 19:57 UTC (permalink / raw)
To: David Howells; +Cc: LKML, Al Viro, jose.bollo, Linux Security Module list
In-Reply-To: <9986.1559069658@warthog.procyon.org.uk>
On 5/28/2019 11:54 AM, David Howells wrote:
> Casey Schaufler <casey@schaufler-ca.com> wrote:
>
>>> Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>
>>>>> Also, should all of these be prefixed with "smack"? So:
>>>>>
>>>>> fsparam_string("smackfsdef", Opt_fsdefault),
>>>>> fsparam_string("smackfsfloor", Opt_fsfloor),
>>>>> fsparam_string("smackfshat", Opt_fshat),
>>>> No. smack_fs_parameters takes care of that.
>>> It does? *Blink*.
>> Well, something does. I can't say that I 100% understand all
>> of how the new mount code handles the mount options. Y'all made
>> sweeping changes, and the code works the way it used to except
>> for the awkward change from smackfsdef to smackfsdefault. It
>> took no small amount of head scratching and experimentation to
>> convince myself that the fix I proposed was correct.
> Ah... I suspect the issue is that smack_sb_eat_lsm_opts() strips the prefix
> for an unconverted filesystem, but smack_fs_context_parse_param() doesn't
> (which it shouldn't).
>
> Can you try grabbing my mount-api-viro branch from:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
>
> and testing setting smack options on a tmpfs filesystem?
My fedora system won't boot because smackfsdef isn't recognized. :(
I will put in my fix and retry.
>
> You might need to try modifying samples/vfs/test-fsmount.c to make it mount a
> trmpfs filesystem through the new mount UAPI.
>
> David
^ permalink raw reply
* Re: [PULL] Smack: Restore the smackfsdef mount option
From: Casey Schaufler @ 2019-05-28 19:43 UTC (permalink / raw)
To: David Howells
Cc: James Morris, Linux Security Module list, Al Viro, LKML, casey
In-Reply-To: <10710.1559070135@warthog.procyon.org.uk>
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
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557835765.4139.9.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>>
>> @@ -326,6 +356,10 @@ int ima_appraise_measurement(enum ima_hooks func,
>> case INTEGRITY_UNKNOWN:
>> break;
>> case INTEGRITY_NOXATTRS:/* No EVM protected xattrs. */
>> +/* It's fine not to have xattrs when using a modsig. */
>> +if (try_modsig)
>> +break;
>> +/* fall through */
>> case INTEGRITY_NOLABEL:/* No security.evm xattr. */
>> cause = "missing-HMAC";
>> goto out;
>> @@ -340,6 +374,14 @@ int ima_appraise_measurement(enum ima_hooks func,
>> rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
>> &cause);
>>
>> +/*
>> + * If we have a modsig and either no imasig or the imasig's key isn't
>> + * known, then try verifying the modsig.
>> + */
>> +if (status != INTEGRITY_PASS && try_modsig &&
>> + (!xattr_value || rc == -ENOKEY))
>> +rc = modsig_verify(func, modsig, &status, &cause);
>
> EVM protects other security xattrs, not just security.ima, if they
> exist. As a result, evm_verifyxattr() could pass based on the other
> security xattrs.
Indeed! It doesn't make sense to test for status != INTEGRITY_PASS here.
Not sure what I was thinking. Thanks for spotting it. With your other
comments about this if clause, this code now reads:
/*
* If we have a modsig and either no imasig or the imasig's key isn't
* known, then try verifying the modsig.
*/
if (try_modsig &&
(!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
rc == -ENOKEY))
rc = modsig_verify(func, modsig, &status, &cause);
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:23 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442868.10635.87.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index fca7a3f23321..a7a20a8c15c1 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
>> }
>> }
>>
>> +#define __ima_hook_stringify(str) (#str),
>> +
>> +const char *const func_tokens[] = {
>> + __ima_hooks(__ima_hook_stringify)
>> +};
>> +
>> #ifdef CONFIG_IMA_READ_POLICY
>> enum {
>> mask_exec = 0, mask_write, mask_read, mask_append
>> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
>> "MAY_APPEND"
>> };
>>
>> -#define __ima_hook_stringify(str) (#str),
>> -
>> -static const char *const func_tokens[] = {
>> - __ima_hooks(__ima_hook_stringify)
>> -};
>> -
>> void *ima_policy_start(struct seq_file *m, loff_t *pos)
>> {
>> loff_t l = *pos;
>
> Is moving this something left over from previous versions or there is
> a need for this change?
Well, it's not a strong need, but it's still relevant in the current
version. I use func_tokens in ima_read_modsig() in order to be able to
mention the hook name in mod_check_sig()'s error message:
In ima_read_modsig():
rc = mod_check_sig(sig, buf_len, func_tokens[func]);
And in mod_check_sig():
pr_err("%s: Module is not signed with expected PKCS#7 message\n",
name);
If you think it's not worth it to expose func_tokens, I can make
ima_read_modsig() pass a more generic const string such as "IMA modsig"
for example.
> Other than this, the patch looks good.
Nice!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: Jann Horn @ 2019-05-28 19:14 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, raven, linux-fsdevel, Linux API, linux-block, keyrings,
linux-security-module, kernel list
In-Reply-To: <155905931502.7587.11705449537368497489.stgit@warthog.procyon.org.uk>
On Tue, May 28, 2019 at 6:03 PM David Howells <dhowells@redhat.com> wrote:
> Implement a misc device that implements a general notification queue as a
> ring buffer that can be mmap()'d from userspace.
[...]
> +receive notifications from the kernel. This is can be used in conjunction
typo: s/is can/can/
[...]
> +Overview
> +========
> +
> +This facility appears as a misc device file that is opened and then mapped and
> +polled. Each time it is opened, it creates a new buffer specific to the
> +returned file descriptor. Then, when the opening process sets watches, it
> +indicates that particular buffer it wants notifications from that watch to be
> +written into. Note that there are no read() and write() methods (except for
s/that particular buffer/the particular buffer/
> +debugging). The user is expected to access the ring directly and to use poll
> +to wait for new data.
[...]
> +/**
> + * __post_watch_notification - Post an event notification
> + * @wlist: The watch list to post the event to.
> + * @n: The notification record to post.
> + * @cred: The creds of the process that triggered the notification.
> + * @id: The ID to match on the watch.
> + *
> + * Post a notification of an event into a set of watch queues and let the users
> + * know.
> + *
> + * If @n is NULL then WATCH_INFO_LENGTH will be set on the next event posted.
> + *
> + * The size of the notification should be set in n->info & WATCH_INFO_LENGTH and
> + * should be in units of sizeof(*n).
> + */
> +void __post_watch_notification(struct watch_list *wlist,
> + struct watch_notification *n,
> + const struct cred *cred,
> + u64 id)
> +{
> + const struct watch_filter *wf;
> + struct watch_queue *wqueue;
> + struct watch *watch;
> +
> + rcu_read_lock();
> +
> + hlist_for_each_entry_rcu(watch, &wlist->watchers, list_node) {
> + if (watch->id != id)
> + continue;
> + n->info &= ~(WATCH_INFO_ID | WATCH_INFO_OVERRUN);
> + n->info |= watch->info_id;
> +
> + wqueue = rcu_dereference(watch->queue);
> + wf = rcu_dereference(wqueue->filter);
> + if (wf && !filter_watch_notification(wf, n))
> + continue;
> +
> + post_one_notification(wqueue, n, cred);
> + }
> +
> + rcu_read_unlock();
> +}
> +EXPORT_SYMBOL(__post_watch_notification);
[...]
> +static vm_fault_t watch_queue_fault(struct vm_fault *vmf)
> +{
> + struct watch_queue *wqueue = vmf->vma->vm_file->private_data;
> + struct page *page;
> +
> + page = wqueue->pages[vmf->pgoff];
I don't see you setting any special properties on the VMA that would
prevent userspace from extending its size via mremap() - no
VM_DONTEXPAND or VM_PFNMAP. So I think you might get an out-of-bounds
access here?
> + get_page(page);
> + if (!lock_page_or_retry(page, vmf->vma->vm_mm, vmf->flags)) {
> + put_page(page);
> + return VM_FAULT_RETRY;
> + }
> + vmf->page = page;
> + return VM_FAULT_LOCKED;
> +}
> +
> +static void watch_queue_map_pages(struct vm_fault *vmf,
> + pgoff_t start_pgoff, pgoff_t end_pgoff)
> +{
> + struct watch_queue *wqueue = vmf->vma->vm_file->private_data;
> + struct page *page;
> +
> + rcu_read_lock();
> +
> + do {
> + page = wqueue->pages[start_pgoff];
Same as above.
> + if (trylock_page(page)) {
> + vm_fault_t ret;
> + get_page(page);
> + ret = alloc_set_pte(vmf, NULL, page);
> + if (ret != 0)
> + put_page(page);
> +
> + unlock_page(page);
> + }
> + } while (++start_pgoff < end_pgoff);
> +
> + rcu_read_unlock();
> +}
[...]
> +static int watch_queue_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct watch_queue *wqueue = file->private_data;
> +
> + if (vma->vm_pgoff != 0 ||
> + vma->vm_end - vma->vm_start > wqueue->nr_pages * PAGE_SIZE ||
> + !(pgprot_val(vma->vm_page_prot) & pgprot_val(PAGE_SHARED)))
> + return -EINVAL;
This thing should probably have locking against concurrent
watch_queue_set_size()?
> + vma->vm_ops = &watch_queue_vm_ops;
> +
> + vma_interval_tree_insert(vma, &wqueue->mapping.i_mmap);
> + return 0;
> +}
> +
> +/*
> + * Allocate the required number of pages.
> + */
> +static long watch_queue_set_size(struct watch_queue *wqueue, unsigned long nr_pages)
> +{
> + struct watch_queue_buffer *buf;
> + u32 len;
> + int i;
> +
> + if (nr_pages == 0 ||
> + nr_pages > 16 || /* TODO: choose a better hard limit */
> + !is_power_of_2(nr_pages))
> + return -EINVAL;
> +
> + wqueue->pages = kcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL);
> + if (!wqueue->pages)
> + goto err;
> +
> + for (i = 0; i < nr_pages; i++) {
> + wqueue->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> + if (!wqueue->pages[i])
> + goto err_some_pages;
> + wqueue->pages[i]->mapping = &wqueue->mapping;
> + SetPageUptodate(wqueue->pages[i]);
> + }
> +
> + buf = vmap(wqueue->pages, nr_pages, VM_MAP, PAGE_SHARED);
> + if (!buf)
> + goto err_some_pages;
> +
> + wqueue->buffer = buf;
> + wqueue->nr_pages = nr_pages;
> + wqueue->size = ((nr_pages * PAGE_SIZE) / sizeof(struct watch_notification));
> +
> + /* The first four slots in the buffer contain metadata about the ring,
> + * including the head and tail indices and mask.
> + */
> + len = sizeof(buf->meta) / sizeof(buf->slots[0]);
> + buf->meta.watch.info = len << WATCH_LENGTH_SHIFT;
> + buf->meta.watch.type = WATCH_TYPE_META;
> + buf->meta.watch.subtype = WATCH_META_SKIP_NOTIFICATION;
> + buf->meta.tail = len;
> + buf->meta.mask = wqueue->size - 1;
> + smp_store_release(&buf->meta.head, len);
Why is this an smp_store_release()? The entire buffer should not be visible to
userspace before this setup is complete, right?
> + return 0;
> +
> +err_some_pages:
> + for (i--; i >= 0; i--) {
> + ClearPageUptodate(wqueue->pages[i]);
> + wqueue->pages[i]->mapping = NULL;
> + put_page(wqueue->pages[i]);
> + }
> +
> + kfree(wqueue->pages);
> + wqueue->pages = NULL;
> +err:
> + return -ENOMEM;
> +}
[...]
> +
> +/*
> + * Set parameters.
> + */
> +static long watch_queue_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct watch_queue *wqueue = file->private_data;
> + struct inode *inode = file_inode(file);
> + long ret;
> +
> + switch (cmd) {
> + case IOC_WATCH_QUEUE_SET_SIZE:
> + if (wqueue->buffer)
> + return -EBUSY;
The preceding check occurs without any locks held and therefore has no
reliable effect. It should probably be moved below the
inode_lock(...).
> + inode_lock(inode);
> + ret = watch_queue_set_size(wqueue, arg);
> + inode_unlock(inode);
> + return ret;
> +
> + case IOC_WATCH_QUEUE_SET_FILTER:
> + inode_lock(inode);
> + ret = watch_queue_set_filter(
> + inode, wqueue,
> + (struct watch_notification_filter __user *)arg);
> + inode_unlock(inode);
> + return ret;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
[...]
> +static void free_watch(struct rcu_head *rcu)
> +{
> + struct watch *watch = container_of(rcu, struct watch, rcu);
> +
> + put_watch_queue(rcu_access_pointer(watch->queue));
This should be rcu_dereference_protected(..., 1).
> +/**
> + * remove_watch_from_object - Remove a watch or all watches from an object.
> + * @wlist: The watch list to remove from
> + * @wq: The watch queue of interest (ignored if @all is true)
> + * @id: The ID of the watch to remove (ignored if @all is true)
> + * @all: True to remove all objects
> + *
> + * Remove a specific watch or all watches from an object. A notification is
> + * sent to the watcher to tell them that this happened.
> + */
> +int remove_watch_from_object(struct watch_list *wlist, struct watch_queue *wq,
> + u64 id, bool all)
> +{
> + struct watch_notification n;
> + struct watch_queue *wqueue;
> + struct watch *watch;
> + int ret = -EBADSLT;
> +
> + rcu_read_lock();
> +
> +again:
> + spin_lock(&wlist->lock);
> + hlist_for_each_entry(watch, &wlist->watchers, list_node) {
> + if (all ||
> + (watch->id == id && rcu_access_pointer(watch->queue) == wq))
> + goto found;
> + }
> + spin_unlock(&wlist->lock);
> + goto out;
> +
> +found:
> + ret = 0;
> + hlist_del_init_rcu(&watch->list_node);
> + rcu_assign_pointer(watch->watch_list, NULL);
> + spin_unlock(&wlist->lock);
> +
> + n.type = WATCH_TYPE_META;
> + n.subtype = WATCH_META_REMOVAL_NOTIFICATION;
> + n.info = watch->info_id | sizeof(n);
> +
> + wqueue = rcu_dereference(watch->queue);
> + post_one_notification(wqueue, &n, wq ? wq->cred : NULL);
> +
> + /* We don't need the watch list lock for the next bit as RCU is
> + * protecting everything from being deallocated.
Does "everything" mean "the wqueue" or more than that?
> + */
> + if (wqueue) {
> + spin_lock_bh(&wqueue->lock);
> +
> + if (!hlist_unhashed(&watch->queue_node)) {
> + hlist_del_init_rcu(&watch->queue_node);
> + put_watch(watch);
> + }
> +
> + spin_unlock_bh(&wqueue->lock);
> + }
> +
> + if (wlist->release_watch) {
> + rcu_read_unlock();
> + wlist->release_watch(wlist, watch);
> + rcu_read_lock();
> + }
> + put_watch(watch);
> +
> + if (all && !hlist_empty(&wlist->watchers))
> + goto again;
> +out:
> + rcu_read_unlock();
> + return ret;
> +}
> +EXPORT_SYMBOL(remove_watch_from_object);
> +
> +/*
> + * Remove all the watches that are contributory to a queue. This will
> + * potentially race with removal of the watches by the destruction of the
> + * objects being watched or the distribution of notifications.
> + */
> +static void watch_queue_clear(struct watch_queue *wqueue)
> +{
> + struct watch_list *wlist;
> + struct watch *watch;
> + bool release;
> +
> + rcu_read_lock();
> + spin_lock_bh(&wqueue->lock);
> +
> + /* Prevent new additions and prevent notifications from happening */
> + wqueue->defunct = true;
> +
> + while (!hlist_empty(&wqueue->watches)) {
> + watch = hlist_entry(wqueue->watches.first, struct watch, queue_node);
> + hlist_del_init_rcu(&watch->queue_node);
> + spin_unlock_bh(&wqueue->lock);
> +
> + /* We can't do the next bit under the queue lock as we need to
> + * get the list lock - which would cause a deadlock if someone
> + * was removing from the opposite direction at the same time or
> + * posting a notification.
> + */
> + wlist = rcu_dereference(watch->watch_list);
> + if (wlist) {
> + spin_lock(&wlist->lock);
> +
> + release = !hlist_unhashed(&watch->list_node);
> + if (release) {
> + hlist_del_init_rcu(&watch->list_node);
> + rcu_assign_pointer(watch->watch_list, NULL);
> + }
> +
> + spin_unlock(&wlist->lock);
> +
> + if (release) {
> + if (wlist->release_watch) {
> + rcu_read_unlock();
> + /* This might need to call dput(), so
> + * we have to drop all the locks.
> + */
> + wlist->release_watch(wlist, watch);
How are you holding a reference to `wlist` here? You got the reference through
rcu_dereference(), you've dropped the RCU read lock, and I don't see anything
that stabilizes the reference.
> + rcu_read_lock();
> + }
> + put_watch(watch);
> + }
> + }
> +
> + put_watch(watch);
> + spin_lock_bh(&wqueue->lock);
> + }
> +
> + spin_unlock_bh(&wqueue->lock);
> + rcu_read_unlock();
> +}
> +
> +/*
> + * Release the file.
> + */
> +static int watch_queue_release(struct inode *inode, struct file *file)
> +{
> + struct watch_filter *wfilter;
> + struct watch_queue *wqueue = file->private_data;
> + int i, pgref;
> +
> + watch_queue_clear(wqueue);
> +
> + if (wqueue->pages && wqueue->pages[0])
> + WARN_ON(page_ref_count(wqueue->pages[0]) != 1);
Is there a reason why there couldn't still be references to the pages
from get_user_pages()/get_user_pages_fast()?
> + if (wqueue->buffer)
> + vfree(wqueue->buffer);
> + for (i = 0; i < wqueue->nr_pages; i++) {
> + ClearPageUptodate(wqueue->pages[i]);
> + wqueue->pages[i]->mapping = NULL;
> + pgref = page_ref_count(wqueue->pages[i]);
> + WARN(pgref != 1,
> + "FREE PAGE[%d] refcount %d\n", i, page_ref_count(wqueue->pages[i]));
> + __free_page(wqueue->pages[i]);
> + }
> +
> + wfilter = rcu_access_pointer(wqueue->filter);
Again, rcu_dereference_protected(..., 1).
> + if (wfilter)
> + kfree_rcu(wfilter, rcu);
> + kfree(wqueue->pages);
> + put_cred(wqueue->cred);
> + put_watch_queue(wqueue);
> + return 0;
> +}
> +
> +#ifdef DEBUG_WITH_WRITE
> +static ssize_t watch_queue_write(struct file *file,
> + const char __user *_buf, size_t len, loff_t *pos)
> +{
> + struct watch_notification *n;
> + struct watch_queue *wqueue = file->private_data;
> + ssize_t ret;
> +
> + if (!wqueue->buffer)
> + return -ENOBUFS;
> +
> + if (len & ~WATCH_INFO_LENGTH || len == 0 || !_buf)
> + return -EINVAL;
> +
> + n = memdup_user(_buf, len);
> + if (IS_ERR(n))
> + return PTR_ERR(n);
> +
> + ret = -EINVAL;
> + if ((n->info & WATCH_INFO_LENGTH) != len)
> + goto error;
> + n->info &= (WATCH_INFO_LENGTH | WATCH_INFO_TYPE_FLAGS | WATCH_INFO_ID);
Should the non-atomic modification of n->info (and perhaps also the
following uses of ->debug) be protected by some lock?
> + if (post_one_notification(wqueue, n, file->f_cred))
> + wqueue->debug = 0;
> + else
> + wqueue->debug++;
> + ret = len;
> + if (wqueue->debug > 20)
> + ret = -EIO;
> +
> +error:
> + kfree(n);
> + return ret;
> +}
> +#endif
[...]
> +#define IOC_WATCH_QUEUE_SET_SIZE _IO('s', 0x01) /* Set the size in pages */
> +#define IOC_WATCH_QUEUE_SET_FILTER _IO('s', 0x02) /* Set the filter */
Should these ioctl numbers be registered in
Documentation/ioctl/ioctl-number.txt?
^ permalink raw reply
* Re: [PATCH v10 12/12] ima: Store the measurement again when appraising a modsig
From: Thiago Jung Bauermann @ 2019-05-28 19:14 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1559052560.4090.14.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> If the IMA template contains the "modsig" or "d-modsig" field, then the
>> modsig should be added to the measurement list when the file is appraised.
>>
>> And that is what normally happens, but if a measurement rule caused a file
>> containing a modsig to be measured before a different rule causes it to be
>> appraised, the resulting measurement entry will not contain the modsig
>> because it is only fetched during appraisal. When the appraisal rule
>> triggers, it won't store a new measurement containing the modsig because
>> the file was already measured.
>>
>> We need to detect that situation and store an additional measurement with
>> the modsig. This is done by adding an IMA_MEASURE action flag if we read a
>> modsig and the IMA template contains a modsig field.
>
> With the new per policy rule "template" support being added, this
> patch needs to be modified so that the per policy "template" format is
> checked. ima_template_has_modsig() should be called with the
> template_desc being used.
Right. Thanks for point out what needs to be done. After rebasing on top
of Matthew Garret's "IMA: Allow profiles to define the desired IMA
template" patch I changed ima_template_has_modsig() to check the
template_desc obtained from process_measurement().
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 11/12] ima: Define ima-modsig template
From: Thiago Jung Bauermann @ 2019-05-28 19:09 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442889.10635.88.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> Define new "d-modsig" template field which holds the digest that is
>> expected to match the one contained in the modsig, and also new "modsig"
>> template field which holds the appended file signature.
>>
>> Add a new "ima-modsig" defined template descriptor with the new fields as
>> well as the ones from the "ima-sig" descriptor.
>>
>> Change ima_store_measurement() to accept a struct modsig * argument so that
>> it can be passed along to the templates via struct ima_event_data.
>>
>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Thanks, Roberto. Just some thoughts inline below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks!
>> +/*
>> + * Validating the appended signature included in the measurement list requires
>> + * the file hash calculated without the appended signature (i.e., the 'd-modsig'
>> + * field). Therefore, notify the user if they have the 'modsig' field but not
>> + * the 'd-modsig' field in the template.
>> + */
>> +static void check_current_template_modsig(void)
>> +{
>> +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
>> + struct ima_template_desc *template;
>> + bool has_modsig, has_dmodsig;
>> + static bool checked;
>> + int i;
>> +
>> + /* We only need to notify the user once. */
>> + if (checked)
>> + return;
>> +
>> + has_modsig = has_dmodsig = false;
>> + template = ima_template_desc_current();
>> + for (i = 0; i < template->num_fields; i++) {
>> + if (!strcmp(template->fields[i]->field_id, "modsig"))
>> + has_modsig = true;
>> + else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
>> + has_dmodsig = true;
>> + }
>> +
>> + if (has_modsig && !has_dmodsig)
>> + pr_notice(MSG);
>> +
>> + checked = true;
>> +#undef MSG
>> +}
>> +
>
> There was some recent discussion about supporting per IMA policy rule
> template formats. This feature will allow just the kexec kernel image
> to require ima-modsig. When per policy rule template formats support
> is upstreamed, this function will need to be updated.
Indeed. Thanks for the clarification. For the next iteration I rebased
on top of Matthew Garret's "IMA: Allow profiles to define the desired
IMA template" patch. I'm currently adapting this check accordingly.
>> @@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
>> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
>> DATA_FMT_HEX, field_data);
>> }
>> +
>> +int ima_eventmodsig_init(struct ima_event_data *event_data,
>> + struct ima_field_data *field_data)
>> +{
>> + const void *data;
>> + u32 data_len;
>> + int rc;
>> +
>> + if (!event_data->modsig)
>> + return 0;
>> +
>> + /*
>> + * The xattr_value for IMA_MODSIG is a runtime structure containing
>> + * pointers. Get its raw data instead.
>> + */
>
> "xattr_value"? The comment needs some clarification.
Oops, forgot to update this comment. This is the new version:
/*
* modsig is a runtime structure containing pointers. Get its raw data
* instead.
*/
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 01/12] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2019-05-28 19:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557416528.10635.62.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> IMA will use the module_signature format for append signatures, so export
>> the relevant definitions and factor out the code which verifies that the
>> appended signature trailer is valid.
>>
>> Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
>> and be able to use mod_check_sig() without having to depend on either
>> CONFIG_MODULE_SIG or CONFIG_MODULES.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Cc: Jessica Yu <jeyu@kernel.org>
>
> Just a couple minor questions/comments below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks for your review and your comments!
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 4592bf7997c0..a71019553ee1 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1906,7 +1906,7 @@ config MODULE_SRCVERSION_ALL
>> config MODULE_SIG
>> bool "Module signature verification"
>> depends on MODULES
>> - select SYSTEM_DATA_VERIFICATION
>> + select MODULE_SIG_FORMAT
>> help
>> Check modules for valid signatures upon load: the signature
>> is simply appended to the module. For more information see
>> @@ -2036,6 +2036,10 @@ config TRIM_UNUSED_KSYMS
>>
>> endif # MODULES
>>
>> +config MODULE_SIG_FORMAT
>> + def_bool n
>> + select SYSTEM_DATA_VERIFICATION
>
> Normally Kconfigs, in the same file, are defined before they are used.
> I'm not sure if that is required or just a convention.
I think it's a convention, because it seemed to work in the current way.
For the next version I moved the config MODULE_SIG_FORMAT definition to
just before "menuconfig MODULES"
>> diff --git a/kernel/module_signature.c b/kernel/module_signature.c
>> new file mode 100644
>> index 000000000000..6d5e59f27f55
>> --- /dev/null
>> +++ b/kernel/module_signature.c
>> @@ -0,0 +1,45 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Module signature checker
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
>> + * Written by David Howells (dhowells@redhat.com)
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/printk.h>
>> +#include <linux/module_signature.h>
>> +#include <asm/byteorder.h>
>> +
>> +/**
>> + * mod_check_sig - check that the given signature is sane
>> + *
>> + * @ms: Signature to check.
>> + * @file_len: Size of the file to which @ms is appended.
>
> "name" is missing.
Fixed.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] apparmor: enforce nullbyte at end of tag string
From: John Johansen @ 2019-05-28 19:05 UTC (permalink / raw)
To: Jann Horn; +Cc: Serge E. Hallyn, linux-security-module, linux-kernel
In-Reply-To: <20190528153226.248785-1-jannh@google.com>
On 5/28/19 8:32 AM, Jann Horn wrote:
> A packed AppArmor policy contains null-terminated tag strings that are read
> by unpack_nameX(). However, unpack_nameX() uses string functions on them
> without ensuring that they are actually null-terminated, potentially
> leading to out-of-bounds accesses.
>
> Make sure that the tag string is null-terminated before passing it to
> strcmp().
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
gah! yes!
Acked-by: John Johansen <john.johansen@canonical.com>
> ---
> Warning: The existence of this bug has not been verified at runtime, and
> the patch is compile-tested only. I noticed this while browsing through
> the code, but didn't want to spend the time necessary to figure out how
> to actually test this at runtime.
>
>
> security/apparmor/policy_unpack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index f6c2bcb2ab14..33041c4fb69f 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -276,7 +276,7 @@ static bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
> char *tag = NULL;
> size_t size = unpack_u16_chunk(e, &tag);
> /* if a name is specified it must match. otherwise skip tag */
> - if (name && (!size || strcmp(name, tag)))
> + if (name && (!size || tag[size-1] != '\0' || strcmp(name, tag)))
> goto fail;
> } else if (name) {
> /* if a name is specified and there is no name tag fail */
>
^ permalink raw reply
* Re: [PULL] Smack: Restore the smackfsdef mount option
From: David Howells @ 2019-05-28 19:02 UTC (permalink / raw)
To: Casey Schaufler
Cc: dhowells, James Morris, Linux Security Module list, Al Viro, LKML
In-Reply-To: <6889f4f9-4ae0-8a92-a2fc-04151ad8ed9f@schaufler-ca.com>
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
^ permalink raw reply
* Re: [PATCH] Smack: Restore the smackfsdef mount option
From: David Howells @ 2019-05-28 18:54 UTC (permalink / raw)
To: Casey Schaufler
Cc: dhowells, LKML, Al Viro, jose.bollo, Linux Security Module list
In-Reply-To: <43e3de52-13d7-8089-11cf-a384662401aa@schaufler-ca.com>
Casey Schaufler <casey@schaufler-ca.com> wrote:
> > Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> >>> Also, should all of these be prefixed with "smack"? So:
> >>>
> >>> fsparam_string("smackfsdef", Opt_fsdefault),
> >>> fsparam_string("smackfsfloor", Opt_fsfloor),
> >>> fsparam_string("smackfshat", Opt_fshat),
> >> No. smack_fs_parameters takes care of that.
> > It does? *Blink*.
>
> Well, something does. I can't say that I 100% understand all
> of how the new mount code handles the mount options. Y'all made
> sweeping changes, and the code works the way it used to except
> for the awkward change from smackfsdef to smackfsdefault. It
> took no small amount of head scratching and experimentation to
> convince myself that the fix I proposed was correct.
Ah... I suspect the issue is that smack_sb_eat_lsm_opts() strips the prefix
for an unconverted filesystem, but smack_fs_context_parse_param() doesn't
(which it shouldn't).
Can you try grabbing my mount-api-viro branch from:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
and testing setting smack options on a tmpfs filesystem?
You might need to try modifying samples/vfs/test-fsmount.c to make it mount a
trmpfs filesystem through the new mount UAPI.
David
^ permalink raw reply
* Re: [PATCH] LSM: Fix formatting errors in SafeSetID LSM docs
From: James Morris @ 2019-05-28 18:44 UTC (permalink / raw)
To: Micah Morton; +Cc: linux-security-module
In-Reply-To: <20190528155844.171547-1-mortonm@chromium.org>
On Tue, 28 May 2019, Micah Morton wrote:
> This fixes the unintended occurrences of ??? in the text.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PULL] Smack: Restore the smackfsdef mount option
From: Casey Schaufler @ 2019-05-28 18:29 UTC (permalink / raw)
To: James Morris, Linux Security Module list
Cc: Al Viro, David Howells, casey, LKML
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(+)
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-28 17:30 UTC (permalink / raw)
To: Greg KH
Cc: dhowells, viro, raven, linux-fsdevel, linux-api, linux-block,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20190528162603.GA24097@kroah.com>
Greg KH <gregkh@linuxfoundation.org> wrote:
> > Implement a misc device that implements a general notification queue as a
> > ring buffer that can be mmap()'d from userspace.
>
> "general" but just for filesystems, right? :(
Whatever gave you that idea? You can watch keyrings events, for example -
they're not exactly filesystems. I've added the ability to watch for mount
topology changes and superblock events because those are something I've been
asked to do. I've added something for block events because I've recently had
a problem with trying to recover data from a dodgy disk in that every time the
disk goes offline, the ddrecover goes "wheeeee!" as it just sees a lot of
EIO/ENODATA at a great rate of knots because it doesn't know the driver is now
ignoring the disk.
I don't know what else people might want to watch, but I've tried to make it
as generic as possible so as not to exclude it if possible.
> This doesn't match the structure definition in the documentation, so
> something is out of sync.
Ah, yes - I need to update that doc, thanks.
> I'm all for a "generic" event system for the kernel (heck, Solaris has
> had one for decades), but it keeps getting shot down every time it comes
> up. What is different about this one?
Without studying all the other ones, I can't say - however, I need to add
something for keyrings and I would prefer to make something generic.
> > +#define DEBUG_WITH_WRITE /* Allow use of write() to record notifications */
>
> debugging code left in?
I'll switch it to #undef. I want to leave the code in there for testing
purposes. Possibly I should make it a Kconfig option.
> > + refcount_t usage;
>
> Usage of what, this structure? Or something else?
This is the number of usages of this struct (references to if you prefer). I
can add a comment to this effect.
> > +EXPORT_SYMBOL(__post_watch_notification);
>
> _GPL for new apis? (I have to ask...)
No.
> > + return -EOPNOTSUPP;
>
> -ENOTTY is the correct "not a valid ioctl" error value, right?
fs/ioctl.c does both, but I can switch it if it makes you happier.
> > +void put_watch_queue(struct watch_queue *wqueue)
> > +{
> > + if (refcount_dec_and_test(&wqueue->usage))
> > + kfree_rcu(wqueue, rcu);
>
> Why not just use a kref?
Why use a kref? It seems like an effort to be a C++ base class, but without
the C++ inheritance bit. Using kref doesn't seem to gain anything. It's just
a wrapper around refcount_t - so why not just use a refcount_t?
kref_put() could potentially add an unnecessary extra stack frame and would
seem to be best avoided, though an optimising compiler ought to be able to
inline if it can.
Are you now on the convert all refcounts to krefs path?
> > +EXPORT_SYMBOL(add_watch_to_object);
>
> Naming nit, shouldn't the "prefix" all be the same for these new
> functions?
>
> watch_queue_add_object()? watch_queue_put()? And so on?
Naming is fun. watch_queue_add_object - that suggests something different to
what the function actually does. I'll think about adjusting the names.
> > +module_exit(watch_queue_exit);
>
> module_misc_device()?
warthog>git grep module_misc_device -- Documentation/
warthog1>
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>
> Yeah!!!
Blech.
> > + struct {
> > + struct watch_notification watch; /* WATCH_TYPE_SKIP */
> > + volatile __u32 head; /* Ring head index */
> > + volatile __u32 tail; /* Ring tail index */
>
> A uapi structure that has volatile in it? Are you _SURE_ this is
> correct?
>
> That feels wrong to me... This is not a backing-hardware register, it's
> "just memory" and slapping volatile on it shouldn't be the correct
> solution for telling the compiler to not to optimize away reads/flushes,
> right? You need a proper memory access type primitive for that to work
> correctly everywhere I thought.
>
> We only have 2 users of volatile in include/uapi, one for WMI structures
> that are backed by firmware (seems correct), and one for DRM which I
> have no idea how it works as it claims to be a lock. Why is this new
> addition the correct way to do this that no other ring-buffer that was
> mmapped has needed to?
Yeah, I understand your concern with this.
The reason I put the volatiles in is that the kernel may be modifying the head
pointer on one CPU simultaneously with userspace modifying the tail pointer on
another CPU.
Note that userspace does not need to enter the kernel to find out if there's
anything in the buffer or to read stuff out of the buffer. Userspace only
needs to enter the kernel, using poll() or similar, to wait for something to
appear in the buffer.
David
^ permalink raw reply
* Re: [PATCH] Smack: Restore the smackfsdef mount option
From: Casey Schaufler @ 2019-05-28 16:41 UTC (permalink / raw)
To: David Howells
Cc: LKML, Al Viro, jose.bollo, Linux Security Module list, casey
In-Reply-To: <9330.1559060541@warthog.procyon.org.uk>
On 5/28/2019 9:22 AM, David Howells wrote:
> Casey Schaufler <casey@schaufler-ca.com> wrote:
>
>>> Also, should all of these be prefixed with "smack"? So:
>>>
>>> fsparam_string("smackfsdef", Opt_fsdefault),
>>> fsparam_string("smackfsfloor", Opt_fsfloor),
>>> fsparam_string("smackfshat", Opt_fshat),
>> No. smack_fs_parameters takes care of that.
> It does? *Blink*.
Well, something does. I can't say that I 100% understand all
of how the new mount code handles the mount options. Y'all made
sweeping changes, and the code works the way it used to except
for the awkward change from smackfsdef to smackfsdefault. It
took no small amount of head scratching and experimentation to
convince myself that the fix I proposed was correct.
>
> smack_fs_parameters.name is just for decorating messages, if that's what
> you're looking at.
>
> David
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: Greg KH @ 2019-05-28 16:26 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155905931502.7587.11705449537368497489.stgit@warthog.procyon.org.uk>
On Tue, May 28, 2019 at 05:01:55PM +0100, David Howells wrote:
> Implement a misc device that implements a general notification queue as a
> ring buffer that can be mmap()'d from userspace.
"general" but just for filesystems, right? :(
> Each entry has a 1-slot header that describes it:
>
> struct watch_notification {
> __u32 type:24;
> __u32 subtype:8;
> __u32 info;
> };
This doesn't match the structure definition in the documentation, so
something is out of sync.
> The type indicates the source (eg. mount tree changes, superblock events,
> keyring changes, block layer events) and the subtype indicates the event
> type (eg. mount, unmount; EIO, EDQUOT; link, unlink). The info field
> indicates a number of things, including the entry length, an ID assigned to
> a watchpoint contributing to this buffer, type-specific flags and meta
> flags, such as an overrun indicator.
>
> Supplementary data, such as the key ID that generated an event, are
> attached in additional slots.
I'm all for a "generic" event system for the kernel (heck, Solaris has
had one for decades), but it keeps getting shot down every time it comes
up. What is different about this one?
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -4,6 +4,19 @@
>
> menu "Misc devices"
>
> +config WATCH_QUEUE
> + bool "Mappable notification queue"
> + default n
Nit, not needed.
> + depends on MMU
> + help
> + This is a general notification queue for the kernel to pass events to
> + userspace through a mmap()'able ring buffer. It can be used in
> + conjunction with watches for mount topology change notifications,
> + superblock change notifications and key/keyring change notifications.
> +
> + Note that in theory this should work fine with NOMMU, but I'm not
> + sure how to make that work.
> +
> config SENSORS_LIS3LV02D
> tristate
> depends on INPUT
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index b9affcdaa3d6..bf16acd9f8cc 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -3,6 +3,7 @@
> # Makefile for misc devices that really don't fit anywhere else.
> #
>
> +obj-$(CONFIG_WATCH_QUEUE) += watch_queue.o
> obj-$(CONFIG_IBM_ASM) += ibmasm/
> obj-$(CONFIG_IBMVMC) += ibmvmc.o
> obj-$(CONFIG_AD525X_DPOT) += ad525x_dpot.o
> diff --git a/drivers/misc/watch_queue.c b/drivers/misc/watch_queue.c
> new file mode 100644
> index 000000000000..39a09ea15d97
> --- /dev/null
> +++ b/drivers/misc/watch_queue.c
> @@ -0,0 +1,877 @@
> +/* User-mappable watch queue
> + *
> + * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
You didn't touch the code this year?
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
Please drop the boiler plate text and use a SPDX tag, checkpatch should
have caught this. I don't want to have to go and change it again.
> + *
> + * See Documentation/watch_queue.rst
> + */
> +
> +#define pr_fmt(fmt) "watchq: " fmt
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/printk.h>
> +#include <linux/miscdevice.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include <linux/pagemap.h>
> +#include <linux/poll.h>
> +#include <linux/uaccess.h>
> +#include <linux/vmalloc.h>
> +#include <linux/file.h>
> +#include <linux/security.h>
> +#include <linux/cred.h>
> +#include <linux/watch_queue.h>
> +
> +#define DEBUG_WITH_WRITE /* Allow use of write() to record notifications */
debugging code left in?
> +
> +MODULE_DESCRIPTION("Watch queue");
> +MODULE_AUTHOR("Red Hat, Inc.");
> +MODULE_LICENSE("GPL");
> +
> +struct watch_type_filter {
> + enum watch_notification_type type;
> + __u32 subtype_filter[1]; /* Bitmask of subtypes to filter on */
> + __u32 info_filter; /* Filter on watch_notification::info */
> + __u32 info_mask; /* Mask of relevant bits in info_filter */
> +};
> +
> +struct watch_filter {
> + union {
> + struct rcu_head rcu;
> + unsigned long type_filter[2]; /* Bitmask of accepted types */
> + };
> + u32 nr_filters; /* Number of filters */
> + struct watch_type_filter filters[];
> +};
> +
> +struct watch_queue {
> + struct rcu_head rcu;
> + struct address_space mapping;
> + const struct cred *cred; /* Creds of the owner of the queue */
> + struct watch_filter __rcu *filter;
> + wait_queue_head_t waiters;
> + struct hlist_head watches; /* Contributory watches */
> + refcount_t usage;
Usage of what, this structure? Or something else?
> + spinlock_t lock;
> + bool defunct; /* T when queues closed */
> + u8 nr_pages; /* Size of pages[] */
> + u8 flag_next; /* Flag to apply to next item */
> +#ifdef DEBUG_WITH_WRITE
> + u8 debug;
> +#endif
> + u32 size;
> + struct watch_queue_buffer *buffer; /* Pointer to first record */
> +
> + /* The mappable pages. The zeroth page holds the ring pointers. */
> + struct page **pages;
> +};
> +EXPORT_SYMBOL(__post_watch_notification);
_GPL for new apis? (I have to ask...)
> +static long watch_queue_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct watch_queue *wqueue = file->private_data;
> + struct inode *inode = file_inode(file);
> + long ret;
> +
> + switch (cmd) {
> + case IOC_WATCH_QUEUE_SET_SIZE:
> + if (wqueue->buffer)
> + return -EBUSY;
> + inode_lock(inode);
> + ret = watch_queue_set_size(wqueue, arg);
> + inode_unlock(inode);
> + return ret;
> +
> + case IOC_WATCH_QUEUE_SET_FILTER:
> + inode_lock(inode);
> + ret = watch_queue_set_filter(
> + inode, wqueue,
> + (struct watch_notification_filter __user *)arg);
> + inode_unlock(inode);
> + return ret;
> +
> + default:
> + return -EOPNOTSUPP;
-ENOTTY is the correct "not a valid ioctl" error value, right?
> + }
> +}
> +/**
> + * put_watch_queue - Dispose of a ref on a watchqueue.
> + * @wqueue: The watch queue to unref.
> + */
> +void put_watch_queue(struct watch_queue *wqueue)
> +{
> + if (refcount_dec_and_test(&wqueue->usage))
> + kfree_rcu(wqueue, rcu);
Why not just use a kref?
> +}
> +EXPORT_SYMBOL(put_watch_queue);
> +int add_watch_to_object(struct watch *watch, struct watch_list *wlist)
> +{
> + struct watch_queue *wqueue = rcu_access_pointer(watch->queue);
> + struct watch *w;
> +
> + hlist_for_each_entry(w, &wlist->watchers, list_node) {
> + if (watch->id == w->id)
> + return -EBUSY;
> + }
> +
> + rcu_assign_pointer(watch->watch_list, wlist);
> +
> + spin_lock_bh(&wqueue->lock);
> + refcount_inc(&wqueue->usage);
> + hlist_add_head(&watch->queue_node, &wqueue->watches);
> + spin_unlock_bh(&wqueue->lock);
> +
> + hlist_add_head(&watch->list_node, &wlist->watchers);
> + return 0;
> +}
> +EXPORT_SYMBOL(add_watch_to_object);
Naming nit, shouldn't the "prefix" all be the same for these new
functions?
watch_queue_add_object()? watch_queue_put()? And so on?
> +static int __init watch_queue_init(void)
> +{
> + int ret;
> +
> + ret = misc_register(&watch_queue_dev);
> + if (ret < 0)
> + pr_err("Failed to register %d\n", ret);
> + return ret;
> +}
> +fs_initcall(watch_queue_init);
> +
> +static void __exit watch_queue_exit(void)
> +{
> + misc_deregister(&watch_queue_dev);
> +}
> +module_exit(watch_queue_exit);
module_misc_device()?
> --- /dev/null
> +++ b/include/linux/watch_queue.h
> @@ -0,0 +1,86 @@
> +/* User-mappable watch queue
> + *
> + * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
Again, SPDX headers please.
> --- /dev/null
> +++ b/include/uapi/linux/watch_queue.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
Yeah!!!
No copyright? :(
> +#ifndef _UAPI_LINUX_WATCH_QUEUE_H
> +#define _UAPI_LINUX_WATCH_QUEUE_H
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +#define IOC_WATCH_QUEUE_SET_SIZE _IO('s', 0x01) /* Set the size in pages */
> +#define IOC_WATCH_QUEUE_SET_FILTER _IO('s', 0x02) /* Set the filter */
> +
> +enum watch_notification_type {
> + WATCH_TYPE_META = 0, /* Special record */
> + WATCH_TYPE_MOUNT_NOTIFY = 1, /* Mount notification record */
> + WATCH_TYPE_SB_NOTIFY = 2, /* Superblock notification */
> + WATCH_TYPE_KEY_NOTIFY = 3, /* Key/keyring change notification */
> + WATCH_TYPE_BLOCK_NOTIFY = 4, /* Block layer notifications */
> +#define WATCH_TYPE___NR 5
> +};
> +
> +enum watch_meta_notification_subtype {
> + WATCH_META_SKIP_NOTIFICATION = 0, /* Just skip this record */
> + WATCH_META_REMOVAL_NOTIFICATION = 1, /* Watched object was removed */
> +};
> +
> +/*
> + * Notification record
> + */
> +struct watch_notification {
> + __u32 type:24; /* enum watch_notification_type */
> + __u32 subtype:8; /* Type-specific subtype (filterable) */
> + __u32 info;
> +#define WATCH_INFO_OVERRUN 0x00000001 /* Event(s) lost due to overrun */
> +#define WATCH_INFO_ENOMEM 0x00000002 /* Event(s) lost due to ENOMEM */
> +#define WATCH_INFO_RECURSIVE 0x00000004 /* Change was recursive */
> +#define WATCH_INFO_LENGTH 0x000001f8 /* Length of record / sizeof(watch_notification) */
> +#define WATCH_INFO_IN_SUBTREE 0x00000200 /* Change was not at watched root */
> +#define WATCH_INFO_TYPE_FLAGS 0x00ff0000 /* Type-specific flags */
> +#define WATCH_INFO_FLAG_0 0x00010000
> +#define WATCH_INFO_FLAG_1 0x00020000
> +#define WATCH_INFO_FLAG_2 0x00040000
> +#define WATCH_INFO_FLAG_3 0x00080000
> +#define WATCH_INFO_FLAG_4 0x00100000
> +#define WATCH_INFO_FLAG_5 0x00200000
> +#define WATCH_INFO_FLAG_6 0x00400000
> +#define WATCH_INFO_FLAG_7 0x00800000
> +#define WATCH_INFO_ID 0xff000000 /* ID of watchpoint */
> +};
> +
> +#define WATCH_LENGTH_SHIFT 3
> +
> +struct watch_queue_buffer {
> + union {
> + /* The first few entries are special, containing the
> + * ring management variables.
> + */
> + struct {
> + struct watch_notification watch; /* WATCH_TYPE_SKIP */
> + volatile __u32 head; /* Ring head index */
> + volatile __u32 tail; /* Ring tail index */
A uapi structure that has volatile in it? Are you _SURE_ this is
correct?
That feels wrong to me... This is not a backing-hardware register, it's
"just memory" and slapping volatile on it shouldn't be the correct
solution for telling the compiler to not to optimize away reads/flushes,
right? You need a proper memory access type primitive for that to work
correctly everywhere I thought.
We only have 2 users of volatile in include/uapi, one for WMI structures
that are backed by firmware (seems correct), and one for DRM which I
have no idea how it works as it claims to be a lock. Why is this new
addition the correct way to do this that no other ring-buffer that was
mmapped has needed to?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Smack: Restore the smackfsdef mount option
From: David Howells @ 2019-05-28 16:22 UTC (permalink / raw)
To: Casey Schaufler
Cc: dhowells, LKML, Al Viro, jose.bollo, Linux Security Module list
In-Reply-To: <9191ef31-a022-cdc4-9bed-ff225e4179bb@schaufler-ca.com>
Casey Schaufler <casey@schaufler-ca.com> wrote:
> > Also, should all of these be prefixed with "smack"? So:
> >
> > fsparam_string("smackfsdef", Opt_fsdefault),
> > fsparam_string("smackfsfloor", Opt_fsfloor),
> > fsparam_string("smackfshat", Opt_fshat),
>
> No. smack_fs_parameters takes care of that.
It does? *Blink*.
smack_fs_parameters.name is just for decorating messages, if that's what
you're looking at.
David
^ permalink raw reply
* [PATCH 7/7] Add sample notification program
From: David Howells @ 2019-05-28 16:02 UTC (permalink / raw)
To: viro
Cc: dhowells, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155905930702.7587.7100265859075976147.stgit@warthog.procyon.org.uk>
This needs to be linked with -lkeyutils.
It is run like:
./watch_test
and watches "/" for mount changes and the current session keyring for key
changes:
# keyctl add user a a @s
1035096409
# keyctl unlink 1035096409 @s
# mount -t tmpfs none /mnt/nfsv3tcp/
# umount /mnt/nfsv3tcp
producing:
# ./watch_test
ptrs h=4 t=2 m=20003
NOTIFY[00000004-00000002] ty=0003 sy=0002 i=01000010
KEY 2ffc2e5d change=2[linked] aux=1035096409
ptrs h=6 t=4 m=20003
NOTIFY[00000006-00000004] ty=0003 sy=0003 i=01000010
KEY 2ffc2e5d change=3[unlinked] aux=1035096409
ptrs h=8 t=6 m=20003
NOTIFY[00000008-00000006] ty=0001 sy=0000 i=02000010
MOUNT 00000013 change=0[new_mount] aux=168
ptrs h=a t=8 m=20003
NOTIFY[0000000a-00000008] ty=0001 sy=0001 i=02000010
MOUNT 00000013 change=1[unmount] aux=168
Other events may be produced, such as with a failing disk:
ptrs h=5 t=2 m=6000004
NOTIFY[00000005-00000002] ty=0004 sy=0006 i=04000018
BLOCK 00800050 e=6[critical medium] s=5be8
This corresponds to:
print_req_error: critical medium error, dev sdf, sector 23528 flags 0
in dmesg.
Signed-off-by: David Howells <dhowells@redhat.com>
---
samples/Kconfig | 6 +
samples/Makefile | 1
samples/watch_queue/Makefile | 9 +
samples/watch_queue/watch_test.c | 284 ++++++++++++++++++++++++++++++++++++++
4 files changed, 300 insertions(+)
create mode 100644 samples/watch_queue/Makefile
create mode 100644 samples/watch_queue/watch_test.c
diff --git a/samples/Kconfig b/samples/Kconfig
index 0561a94f6fdb..a2b7a7babee5 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -160,4 +160,10 @@ config SAMPLE_VFS
as mount API and statx(). Note that this is restricted to the x86
arch whilst it accesses system calls that aren't yet in all arches.
+config SAMPLE_WATCH_QUEUE
+ bool "Build example /dev/watch_queue notification consumer"
+ help
+ Build example userspace program to use the new mount_notify(),
+ sb_notify() syscalls and the KEYCTL_WATCH_KEY keyctl() function.
+
endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index debf8925f06f..ed3b8bab6e9b 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SAMPLE_TRACE_PRINTK) += trace_printk/
obj-$(CONFIG_VIDEO_PCI_SKELETON) += v4l/
obj-y += vfio-mdev/
subdir-$(CONFIG_SAMPLE_VFS) += vfs
+subdir-$(CONFIG_SAMPLE_WATCH_QUEUE) += watch_queue
diff --git a/samples/watch_queue/Makefile b/samples/watch_queue/Makefile
new file mode 100644
index 000000000000..42b694430d0f
--- /dev/null
+++ b/samples/watch_queue/Makefile
@@ -0,0 +1,9 @@
+# List of programs to build
+hostprogs-y := watch_test
+
+# Tell kbuild to always build the programs
+always := $(hostprogs-y)
+
+HOSTCFLAGS_watch_test.o += -I$(objtree)/usr/include
+
+HOSTLOADLIBES_watch_test += -lkeyutils
diff --git a/samples/watch_queue/watch_test.c b/samples/watch_queue/watch_test.c
new file mode 100644
index 000000000000..0bbab492e237
--- /dev/null
+++ b/samples/watch_queue/watch_test.c
@@ -0,0 +1,284 @@
+/* Use /dev/watch_queue to watch for keyring and mount topology changes.
+ *
+ * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <signal.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <dirent.h>
+#include <errno.h>
+#include <sys/wait.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <poll.h>
+#include <limits.h>
+#include <linux/watch_queue.h>
+#include <linux/unistd.h>
+#include <linux/keyctl.h>
+
+#ifndef __NR_mount_notify
+#define __NR_mount_notify -1
+#endif
+#ifndef __NR_sb_notify
+#define __NR_sb_notify -1
+#endif
+#ifndef __NR_block_notify
+#define __NR_block_notify -1
+#endif
+#ifndef KEYCTL_WATCH_KEY
+#define KEYCTL_WATCH_KEY -1
+#endif
+
+#define BUF_SIZE 4
+
+static const char *key_subtypes[256] = {
+ [NOTIFY_KEY_INSTANTIATED] = "instantiated",
+ [NOTIFY_KEY_UPDATED] = "updated",
+ [NOTIFY_KEY_LINKED] = "linked",
+ [NOTIFY_KEY_UNLINKED] = "unlinked",
+ [NOTIFY_KEY_CLEARED] = "cleared",
+ [NOTIFY_KEY_REVOKED] = "revoked",
+ [NOTIFY_KEY_INVALIDATED] = "invalidated",
+ [NOTIFY_KEY_SETATTR] = "setattr",
+};
+
+static void saw_key_change(struct watch_notification *n)
+{
+ struct key_notification *k = (struct key_notification *)n;
+ unsigned int len = n->info & WATCH_INFO_LENGTH;
+
+ if (len != sizeof(struct key_notification))
+ return;
+
+ printf("KEY %08x change=%u[%s] aux=%u\n",
+ k->key_id, n->subtype, key_subtypes[n->subtype], k->aux);
+}
+
+static const char *mount_subtypes[256] = {
+ [NOTIFY_MOUNT_NEW_MOUNT] = "new_mount",
+ [NOTIFY_MOUNT_UNMOUNT] = "unmount",
+ [NOTIFY_MOUNT_EXPIRY] = "expiry",
+ [NOTIFY_MOUNT_READONLY] = "readonly",
+ [NOTIFY_MOUNT_SETATTR] = "setattr",
+ [NOTIFY_MOUNT_MOVE_FROM] = "move_from",
+ [NOTIFY_MOUNT_MOVE_TO] = "move_to",
+};
+
+static long keyctl_watch_key(int key, int watch_fd, int watch_id)
+{
+ return syscall(__NR_keyctl, KEYCTL_WATCH_KEY, key, watch_fd, watch_id);
+}
+
+static void saw_mount_change(struct watch_notification *n)
+{
+ struct mount_notification *m = (struct mount_notification *)n;
+ unsigned int len = n->info & WATCH_INFO_LENGTH;
+
+ if (len != sizeof(struct mount_notification))
+ return;
+
+ printf("MOUNT %08x change=%u[%s] aux=%u\n",
+ m->triggered_on, n->subtype, mount_subtypes[n->subtype], m->changed_mount);
+}
+
+static const char *super_subtypes[256] = {
+ [NOTIFY_SUPERBLOCK_READONLY] = "readonly",
+ [NOTIFY_SUPERBLOCK_ERROR] = "error",
+ [NOTIFY_SUPERBLOCK_EDQUOT] = "edquot",
+ [NOTIFY_SUPERBLOCK_NETWORK] = "network",
+};
+
+static void saw_super_change(struct watch_notification *n)
+{
+ struct superblock_notification *s = (struct superblock_notification *)n;
+ unsigned int len = n->info & WATCH_INFO_LENGTH;
+
+ if (len < sizeof(struct superblock_notification))
+ return;
+
+ printf("SUPER %08llx change=%u[%s]\n",
+ s->sb_id, n->subtype, super_subtypes[n->subtype]);
+}
+
+static const char *block_subtypes[256] = {
+ [NOTIFY_BLOCK_ERROR_TIMEOUT] = "timeout",
+ [NOTIFY_BLOCK_ERROR_NO_SPACE] = "critical space allocation",
+ [NOTIFY_BLOCK_ERROR_RECOVERABLE_TRANSPORT] = "recoverable transport",
+ [NOTIFY_BLOCK_ERROR_CRITICAL_TARGET] = "critical target",
+ [NOTIFY_BLOCK_ERROR_CRITICAL_NEXUS] = "critical nexus",
+ [NOTIFY_BLOCK_ERROR_CRITICAL_MEDIUM] = "critical medium",
+ [NOTIFY_BLOCK_ERROR_PROTECTION] = "protection",
+ [NOTIFY_BLOCK_ERROR_KERNEL_RESOURCE] = "kernel resource",
+ [NOTIFY_BLOCK_ERROR_DEVICE_RESOURCE] = "device resource",
+ [NOTIFY_BLOCK_ERROR_IO] = "I/O",
+};
+
+static void saw_block_change(struct watch_notification *n)
+{
+ struct block_notification *b = (struct block_notification *)n;
+ unsigned int len = n->info & WATCH_INFO_LENGTH;
+
+ if (len < sizeof(struct block_notification))
+ return;
+
+ printf("BLOCK %08llx e=%u[%s] s=%llx\n",
+ (unsigned long long)b->dev,
+ n->subtype, block_subtypes[n->subtype],
+ (unsigned long long)b->sector);
+}
+
+/*
+ * Consume and display events.
+ */
+static int consumer(int fd, struct watch_queue_buffer *buf)
+{
+ struct watch_notification *n;
+ struct pollfd p[1];
+ unsigned int head, tail, mask = buf->meta.mask;
+
+ for (;;) {
+ p[0].fd = fd;
+ p[0].events = POLLIN | POLLERR;
+ p[0].revents = 0;
+
+ if (poll(p, 1, -1) == -1) {
+ perror("poll");
+ break;
+ }
+
+ printf("ptrs h=%x t=%x m=%x\n",
+ buf->meta.head, buf->meta.tail, buf->meta.mask);
+
+ while (head = buf->meta.head,
+ tail = buf->meta.tail,
+ tail != head
+ ) {
+ asm ("lfence" : : : "memory" );
+ n = &buf->slots[tail & mask];
+ printf("NOTIFY[%08x-%08x] ty=%04x sy=%04x i=%08x\n",
+ head, tail, n->type, n->subtype, n->info);
+ if ((n->info & WATCH_INFO_LENGTH) == 0)
+ goto out;
+
+ switch (n->type) {
+ case WATCH_TYPE_META:
+ if (n->subtype == WATCH_META_REMOVAL_NOTIFICATION)
+ printf("REMOVAL of watchpoint %08x\n",
+ n->info & WATCH_INFO_ID);
+ break;
+ case WATCH_TYPE_MOUNT_NOTIFY:
+ saw_mount_change(n);
+ break;
+ case WATCH_TYPE_SB_NOTIFY:
+ saw_super_change(n);
+ break;
+ case WATCH_TYPE_KEY_NOTIFY:
+ saw_key_change(n);
+ break;
+ case WATCH_TYPE_BLOCK_NOTIFY:
+ saw_block_change(n);
+ break;
+ }
+
+ tail += (n->info & WATCH_INFO_LENGTH) >> WATCH_LENGTH_SHIFT;
+ asm("mfence" ::: "memory");
+ buf->meta.tail = tail;
+ }
+ }
+
+out:
+ return 0;
+}
+
+static struct watch_notification_filter filter = {
+ .nr_filters = 4,
+ .__reserved = 0,
+ .filters = {
+ [0] = {
+ .type = WATCH_TYPE_MOUNT_NOTIFY,
+ // Reject move-from notifications
+ .subtype_filter[0] = UINT_MAX & ~(1 << NOTIFY_MOUNT_MOVE_FROM),
+ },
+ [1] = {
+ .type = WATCH_TYPE_SB_NOTIFY,
+ // Only accept notification of changes to R/O state
+ .subtype_filter[0] = (1 << NOTIFY_SUPERBLOCK_READONLY),
+ // Only accept notifications of change-to-R/O
+ .info_mask = WATCH_INFO_FLAG_0,
+ .info_filter = WATCH_INFO_FLAG_0,
+ },
+ [2] = {
+ .type = WATCH_TYPE_KEY_NOTIFY,
+ .subtype_filter[0] = UINT_MAX,
+ },
+ [3] = {
+ .type = WATCH_TYPE_BLOCK_NOTIFY,
+ .subtype_filter[0] = UINT_MAX,
+ },
+ },
+};
+
+int main(int argc, char **argv)
+{
+ struct watch_queue_buffer *buf;
+ size_t page_size;
+ int fd;
+
+ fd = open("/dev/watch_queue", O_RDWR);
+ if (fd == -1) {
+ perror("/dev/watch_queue");
+ exit(1);
+ }
+
+ if (ioctl(fd, IOC_WATCH_QUEUE_SET_SIZE, BUF_SIZE) == -1) {
+ perror("/dev/watch_queue(size)");
+ exit(1);
+ }
+
+ if (ioctl(fd, IOC_WATCH_QUEUE_SET_FILTER, &filter) == -1) {
+ perror("/dev/watch_queue(filter)");
+ exit(1);
+ }
+
+ page_size = sysconf(_SC_PAGESIZE);
+ buf = mmap(NULL, BUF_SIZE * page_size, PROT_READ | PROT_WRITE,
+ MAP_SHARED, fd, 0);
+ if (buf == MAP_FAILED) {
+ perror("mmap");
+ exit(1);
+ }
+
+ if (keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fd, 0x01) == -1) {
+ perror("keyctl");
+ exit(1);
+ }
+
+ if (syscall(__NR_mount_notify, AT_FDCWD, "/", 0, fd, 0x02) == -1) {
+ perror("mount_notify");
+ exit(1);
+ }
+
+ if (syscall(__NR_sb_notify, AT_FDCWD, "/mnt", 0, fd, 0x03) == -1) {
+ perror("sb_notify");
+ exit(1);
+ }
+
+ if (syscall(__NR_block_notify, fd, 0x04) == -1) {
+ perror("block_notify");
+ exit(1);
+ }
+
+ return consumer(fd, buf);
+}
^ permalink raw reply related
* [PATCH 6/7] block: Add block layer notifications
From: David Howells @ 2019-05-28 16:02 UTC (permalink / raw)
To: viro
Cc: dhowells, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155905930702.7587.7100265859075976147.stgit@warthog.procyon.org.uk>
Add a block layer notification mechanism whereby notifications about
block-layer events such as I/O errors, can be reported to a monitoring
process asynchronously.
Firstly, an event queue needs to be created:
fd = open("/dev/event_queue", O_RDWR);
ioctl(fd, IOC_WATCH_QUEUE_SET_SIZE, page_size << n);
then a notification can be set up to report block notifications via that
queue:
struct watch_notification_filter filter = {
.nr_filters = 1,
.filters = {
[0] = {
.type = WATCH_TYPE_BLOCK_NOTIFY,
.subtype_filter[0] = UINT_MAX;
},
},
};
ioctl(fd, IOC_WATCH_QUEUE_SET_FILTER, &filter);
block_notify(fd, 12);
After that, records will be placed into the queue when, for example, errors
occur on a block device. Records are of the following format:
struct block_notification {
struct watch_notification watch;
__u64 dev;
__u64 sector;
} *n;
Where:
n->watch.type will be WATCH_TYPE_BLOCK_NOTIFY
n->watch.subtype will be the type of notification, such as
NOTIFY_BLOCK_ERROR_CRITICAL_MEDIUM.
n->watch.info & WATCH_INFO_LENGTH will indicate the length of the
record.
n->watch.info & WATCH_INFO_ID will be the second argument to
block_notify(), shifted.
n->dev will be the device numbers munged together.
n->sector will indicate the affected sector (if appropriate for the
event).
Note that it is permissible for event records to be of variable length -
or, at least, the length may be dependent on the subtype.
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1
arch/x86/entry/syscalls/syscall_64.tbl | 1
block/Kconfig | 9 +++
block/Makefile | 1
block/blk-core.c | 28 +++++++++++
block/blk-notify.c | 83 ++++++++++++++++++++++++++++++++
include/linux/blkdev.h | 10 ++++
include/linux/syscalls.h | 1
include/uapi/linux/watch_queue.h | 28 +++++++++++
9 files changed, 162 insertions(+)
create mode 100644 block/blk-notify.c
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 429416ce60e1..22793f77c5f1 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -441,3 +441,4 @@
434 i386 fsinfo sys_fsinfo __ia32_sys_fsinfo
435 i386 mount_notify sys_mount_notify __ia32_sys_mount_notify
436 i386 sb_notify sys_sb_notify __ia32_sys_sb_notify
+437 i386 block_notify sys_block_notify __ia32_sys_block_notify
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 4ae146e472db..3f0b82272a9f 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -358,6 +358,7 @@
434 common fsinfo __x64_sys_fsinfo
435 common mount_notify __x64_sys_mount_notify
436 common sb_notify __x64_sys_sb_notify
+437 common block_notify __x64_sys_block_notify
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/block/Kconfig b/block/Kconfig
index 1b220101a9cb..3b0a0ddb83ef 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -163,6 +163,15 @@ config BLK_SED_OPAL
Enabling this option enables users to setup/unlock/lock
Locking ranges for SED devices using the Opal protocol.
+config BLK_NOTIFICATIONS
+ bool "Block layer event notifications"
+ select WATCH_QUEUE
+ help
+ This option provides support for getting block layer event
+ notifications. This makes use of the /dev/watch_queue misc device to
+ handle the notification buffer and provides the block_notify() system
+ call to enable/disable watches.
+
menu "Partition Types"
source "block/partitions/Kconfig"
diff --git a/block/Makefile b/block/Makefile
index eee1b4ceecf9..2dca6273f8f3 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -35,3 +35,4 @@ obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o
obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o
obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o
obj-$(CONFIG_BLK_PM) += blk-pm.o
+obj-$(CONFIG_BLK_NOTIFICATIONS) += blk-notify.o
diff --git a/block/blk-core.c b/block/blk-core.c
index 419d600e6637..8325e33f0bcc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -144,6 +144,21 @@ static const struct {
[BLK_STS_IOERR] = { -EIO, "I/O" },
};
+#ifdef CONFIG_BLK_NOTIFICATIONS
+static const enum block_notification_type blk_notifications[] = {
+ [BLK_STS_TIMEOUT] = NOTIFY_BLOCK_ERROR_TIMEOUT,
+ [BLK_STS_NOSPC] = NOTIFY_BLOCK_ERROR_NO_SPACE,
+ [BLK_STS_TRANSPORT] = NOTIFY_BLOCK_ERROR_RECOVERABLE_TRANSPORT,
+ [BLK_STS_TARGET] = NOTIFY_BLOCK_ERROR_CRITICAL_TARGET,
+ [BLK_STS_NEXUS] = NOTIFY_BLOCK_ERROR_CRITICAL_NEXUS,
+ [BLK_STS_MEDIUM] = NOTIFY_BLOCK_ERROR_CRITICAL_MEDIUM,
+ [BLK_STS_PROTECTION] = NOTIFY_BLOCK_ERROR_PROTECTION,
+ [BLK_STS_RESOURCE] = NOTIFY_BLOCK_ERROR_KERNEL_RESOURCE,
+ [BLK_STS_DEV_RESOURCE] = NOTIFY_BLOCK_ERROR_DEVICE_RESOURCE,
+ [BLK_STS_IOERR] = NOTIFY_BLOCK_ERROR_IO,
+};
+#endif
+
blk_status_t errno_to_blk_status(int errno)
{
int i;
@@ -179,6 +194,19 @@ static void print_req_error(struct request *req, blk_status_t status)
req->rq_disk ? req->rq_disk->disk_name : "?",
(unsigned long long)blk_rq_pos(req),
req->cmd_flags);
+
+#ifdef CONFIG_BLK_NOTIFICATIONS
+ if (blk_notifications[idx]) {
+ struct block_notification n = {
+ .watch.type = WATCH_TYPE_BLOCK_NOTIFY,
+ .watch.subtype = blk_notifications[idx],
+ .watch.info = sizeof(n),
+ .dev = req->rq_disk ? disk_devt(req->rq_disk) : 0,
+ .sector = blk_rq_pos(req),
+ };
+ post_block_notification(&n);
+ }
+#endif
}
static void req_bio_endio(struct request *rq, struct bio *bio,
diff --git a/block/blk-notify.c b/block/blk-notify.c
new file mode 100644
index 000000000000..b310aaf37e7c
--- /dev/null
+++ b/block/blk-notify.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Block layer event notifications.
+ *
+ * Copyright (C) 2019 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#include <linux/blkdev.h>
+#include <linux/watch_queue.h>
+#include <linux/syscalls.h>
+#include <linux/init_task.h>
+
+/*
+ * Global queue for watching for block layer events.
+ */
+static struct watch_list blk_watchers = {
+ .watchers = HLIST_HEAD_INIT,
+ .lock = __SPIN_LOCK_UNLOCKED(&blk_watchers.lock),
+};
+
+static DEFINE_SPINLOCK(blk_watchers_lock);
+
+/*
+ * Post superblock notifications.
+ *
+ * Note that there's only a global queue to which all events are posted. Might
+ * want to provide per-dev queues also.
+ */
+void post_block_notification(struct block_notification *n)
+{
+ u64 id = 0; /* Might want to allow dev# here. */
+
+ post_watch_notification(&blk_watchers, &n->watch, &init_cred, id);
+}
+
+/**
+ * sys_block_notify - Watch for superblock events.
+ * @watch_fd: The watch queue to send notifications to.
+ * @watch_id: The watch ID to be placed in the notification (-1 to remove watch)
+ */
+SYSCALL_DEFINE2(block_notify, int, watch_fd, int, watch_id)
+{
+ struct watch_queue *wqueue;
+ struct watch_list *wlist = &blk_watchers;
+ struct watch *watch;
+ long ret = -ENOMEM;
+ u64 id = 0; /* Might want to allow dev# here. */
+
+ if (watch_id < -1 || watch_id > 0xff)
+ return -EINVAL;
+
+ wqueue = get_watch_queue(watch_fd);
+ if (IS_ERR(wqueue)) {
+ ret = PTR_ERR(wqueue);
+ goto err;
+ }
+
+ if (watch_id >= 0) {
+ watch = kzalloc(sizeof(*watch), GFP_KERNEL);
+ if (!watch)
+ goto err_wqueue;
+
+ init_watch(watch, wqueue);
+ watch->id = id;
+ watch->info_id = (u32)watch_id << WATCH_INFO_ID__SHIFT;
+
+ spin_lock(&blk_watchers_lock);
+ ret = add_watch_to_object(watch, wlist);
+ spin_unlock(&blk_watchers_lock);
+ if (ret < 0)
+ kfree(watch);
+ } else {
+ spin_lock(&blk_watchers_lock);
+ ret = remove_watch_from_object(wlist, wqueue, id, false);
+ spin_unlock(&blk_watchers_lock);
+ }
+
+err_wqueue:
+ put_watch_queue(wqueue);
+err:
+ return ret;
+}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1aafeb923e7b..c28f8647a76d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -43,6 +43,7 @@ struct pr_ops;
struct rq_qos;
struct blk_queue_stats;
struct blk_stat_callback;
+struct block_notification;
#define BLKDEV_MIN_RQ 4
#define BLKDEV_MAX_RQ 128 /* Default maximum */
@@ -1744,6 +1745,15 @@ static inline bool blk_req_can_dispatch_to_zone(struct request *rq)
}
#endif /* CONFIG_BLK_DEV_ZONED */
+#ifdef CONFIG_BLK_NOTIFICATIONS
+extern void post_block_notification(struct block_notification *n);
+#else
+static inline void post_block_notification(struct block_notification *n)
+{
+}
+#endif
+
+
#else /* CONFIG_BLOCK */
struct block_device;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 204a6dbcc34a..77a9d84f1fbd 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1005,6 +1005,7 @@ asmlinkage long sys_mount_notify(int dfd, const char __user *path,
unsigned int at_flags, int watch_fd, int watch_id);
asmlinkage long sys_sb_notify(int dfd, const char __user *path,
unsigned int at_flags, int watch_fd, int watch_id);
+asmlinkage long sys_block_notify(int watch_fd, int watch_id);
/*
* Architecture-specific system calls
diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
index 3b5770889bba..fad276ffa2d0 100644
--- a/include/uapi/linux/watch_queue.h
+++ b/include/uapi/linux/watch_queue.h
@@ -44,6 +44,7 @@ struct watch_notification {
#define WATCH_INFO_FLAG_6 0x00400000
#define WATCH_INFO_FLAG_7 0x00800000
#define WATCH_INFO_ID 0xff000000 /* ID of watchpoint */
+#define WATCH_INFO_ID__SHIFT 24
};
#define WATCH_LENGTH_SHIFT 3
@@ -154,4 +155,31 @@ struct superblock_error_notification {
__u32 error_cookie;
};
+/*
+ * Type of block layer notification.
+ */
+enum block_notification_type {
+ NOTIFY_BLOCK_ERROR_TIMEOUT = 1, /* Timeout error */
+ NOTIFY_BLOCK_ERROR_NO_SPACE = 2, /* Critical space allocation error */
+ NOTIFY_BLOCK_ERROR_RECOVERABLE_TRANSPORT = 3, /* Recoverable transport error */
+ NOTIFY_BLOCK_ERROR_CRITICAL_TARGET = 4, /* Critical target error */
+ NOTIFY_BLOCK_ERROR_CRITICAL_NEXUS = 5, /* Critical nexus error */
+ NOTIFY_BLOCK_ERROR_CRITICAL_MEDIUM = 6, /* Critical medium error */
+ NOTIFY_BLOCK_ERROR_PROTECTION = 7, /* Protection error */
+ NOTIFY_BLOCK_ERROR_KERNEL_RESOURCE = 8, /* Kernel resource error */
+ NOTIFY_BLOCK_ERROR_DEVICE_RESOURCE = 9, /* Device resource error */
+ NOTIFY_BLOCK_ERROR_IO = 10, /* Other I/O error */
+};
+
+/*
+ * Block notification record.
+ * - watch.type = WATCH_TYPE_BLOCK_NOTIFY
+ * - watch.subtype = enum block_notification_type
+ */
+struct block_notification {
+ struct watch_notification watch; /* WATCH_TYPE_SB_NOTIFY */
+ __u64 dev; /* Device number */
+ __u64 sector; /* Affected sector */
+};
+
#endif /* _UAPI_LINUX_WATCH_QUEUE_H */
^ permalink raw reply related
* [PATCH 5/7] fsinfo: Export superblock notification counter
From: David Howells @ 2019-05-28 16:02 UTC (permalink / raw)
To: viro
Cc: dhowells, raven, linux-fsdevel, linux-api, linux-block, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <155905930702.7587.7100265859075976147.stgit@warthog.procyon.org.uk>
Provide an fsinfo attribute to export the superblock notification counter
so that it can be polled in the case of a notification buffer overrun.
This is accessed with:
struct fsinfo_params params = {
.request = FSINFO_ATTR_SB_NOTIFICATIONS,
};
and returns a structure that looks like:
struct fsinfo_sb_notifications {
__u64 watch_id;
__u32 notify_counter;
__u32 __reserved[1];
};
Where watch_id is a number uniquely identifying the superblock in
notification records and notify_counter is incremented for each
superblock notification posted.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/fsinfo.c | 12 ++++++++++++
fs/super.c | 1 +
include/linux/fs.h | 1 +
include/uapi/linux/fsinfo.h | 10 ++++++++++
include/uapi/linux/watch_queue.h | 2 +-
samples/vfs/test-fsinfo.c | 13 +++++++++++++
6 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/fs/fsinfo.c b/fs/fsinfo.c
index 3ec64d3cba08..1456e26d2f7c 100644
--- a/fs/fsinfo.c
+++ b/fs/fsinfo.c
@@ -284,6 +284,16 @@ static int fsinfo_generic_param_enum(struct file_system_type *f,
return sizeof(*p);
}
+static int fsinfo_generic_sb_notifications(struct path *path,
+ struct fsinfo_sb_notifications *p)
+{
+ struct super_block *sb = path->dentry->d_sb;
+
+ p->watch_id = sb->s_unique_id;
+ p->notify_counter = atomic_read(&sb->s_notify_counter);
+ return sizeof(*p);
+}
+
static void fsinfo_insert_sb_flag_parameters(struct path *path,
struct fsinfo_kparams *params)
{
@@ -331,6 +341,7 @@ int generic_fsinfo(struct path *path, struct fsinfo_kparams *params)
case _genp(MOUNT_DEVNAME, mount_devname);
case _genp(MOUNT_CHILDREN, mount_children);
case _genp(MOUNT_SUBMOUNT, mount_submount);
+ case _gen(SB_NOTIFICATIONS, sb_notifications);
default:
return -EOPNOTSUPP;
}
@@ -606,6 +617,7 @@ static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
FSINFO_STRING_N (SERVER_NAME, server_name),
FSINFO_STRUCT_NM (SERVER_ADDRESS, server_address),
FSINFO_STRING (CELL_NAME, cell_name),
+ FSINFO_STRUCT (SB_NOTIFICATIONS, sb_notifications),
};
/**
diff --git a/fs/super.c b/fs/super.c
index 991d69d9dbed..c4bd0d131ef2 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1823,6 +1823,7 @@ EXPORT_SYMBOL(thaw_super);
*/
void post_sb_notification(struct super_block *s, struct superblock_notification *n)
{
+ atomic_inc(&s->s_notify_counter);
post_watch_notification(s->s_watchers, &n->watch, current_cred(),
s->s_unique_id);
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 79ede28f54cc..2c00e292b92b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1535,6 +1535,7 @@ struct super_block {
#ifdef CONFIG_SB_NOTIFICATIONS
struct watch_list *s_watchers;
#endif
+ atomic_t s_notify_counter;
} __randomize_layout;
/* Helper functions so that in most cases filesystems will
diff --git a/include/uapi/linux/fsinfo.h b/include/uapi/linux/fsinfo.h
index 7247088332c2..b4c9446305bb 100644
--- a/include/uapi/linux/fsinfo.h
+++ b/include/uapi/linux/fsinfo.h
@@ -39,6 +39,7 @@ enum fsinfo_attribute {
FSINFO_ATTR_SERVER_NAME = 21, /* Name of the Nth server (string) */
FSINFO_ATTR_SERVER_ADDRESS = 22, /* Mth address of the Nth server */
FSINFO_ATTR_CELL_NAME = 23, /* Cell name (string) */
+ FSINFO_ATTR_SB_NOTIFICATIONS = 24, /* sb_notify() information */
FSINFO_ATTR__NR
};
@@ -308,4 +309,13 @@ struct fsinfo_server_address {
struct __kernel_sockaddr_storage address;
};
+/*
+ * Information struct for fsinfo(FSINFO_ATTR_SB_NOTIFICATIONS).
+ */
+struct fsinfo_sb_notifications {
+ __u64 watch_id; /* Watch ID for superblock. */
+ __u32 notify_counter; /* Number of notifications. */
+ __u32 __reserved[1];
+};
+
#endif /* _UAPI_LINUX_FSINFO_H */
diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
index 126afcc98cc6..3b5770889bba 100644
--- a/include/uapi/linux/watch_queue.h
+++ b/include/uapi/linux/watch_queue.h
@@ -145,7 +145,7 @@ enum superblock_notification_type {
*/
struct superblock_notification {
struct watch_notification watch; /* WATCH_TYPE_SB_NOTIFY */
- __u64 sb_id; /* 64-bit superblock ID [fsinfo_ids::f_sb_id] */
+ __u64 sb_id; /* 64-bit superblock ID [fsinfo_sb_notifications::watch_id] */
};
struct superblock_error_notification {
diff --git a/samples/vfs/test-fsinfo.c b/samples/vfs/test-fsinfo.c
index af29da74559e..0f8f9ded0925 100644
--- a/samples/vfs/test-fsinfo.c
+++ b/samples/vfs/test-fsinfo.c
@@ -90,6 +90,7 @@ static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
FSINFO_STRING_N (SERVER_NAME, server_name),
FSINFO_STRUCT_NM (SERVER_ADDRESS, server_address),
FSINFO_STRING (CELL_NAME, cell_name),
+ FSINFO_STRUCT (SB_NOTIFICATIONS, sb_notifications),
};
#define FSINFO_NAME(X,Y) [FSINFO_ATTR_##X] = #Y
@@ -118,6 +119,7 @@ static const char *fsinfo_attr_names[FSINFO_ATTR__NR] = {
FSINFO_NAME (SERVER_NAME, server_name),
FSINFO_NAME (SERVER_ADDRESS, server_address),
FSINFO_NAME (CELL_NAME, cell_name),
+ FSINFO_NAME (SB_NOTIFICATIONS, sb_notifications),
};
union reply {
@@ -133,6 +135,7 @@ union reply {
struct fsinfo_mount_info mount_info;
struct fsinfo_mount_child mount_children[1];
struct fsinfo_server_address srv_addr;
+ struct fsinfo_sb_notifications sb_notifications;
};
static void dump_hex(unsigned int *data, int from, int to)
@@ -377,6 +380,15 @@ static void dump_attr_MOUNT_CHILDREN(union reply *r, int size)
printf("\t[%u] %8x %8x\n", i++, f->mnt_id, f->notify_counter);
}
+static void dump_attr_SB_NOTIFICATIONS(union reply *r, int size)
+{
+ struct fsinfo_sb_notifications *f = &r->sb_notifications;
+
+ printf("\n");
+ printf("\twatch_id: %llx\n", (unsigned long long)f->watch_id);
+ printf("\tnotifs : %llx\n", (unsigned long long)f->notify_counter);
+}
+
/*
*
*/
@@ -395,6 +407,7 @@ static const dumper_t fsinfo_attr_dumper[FSINFO_ATTR__NR] = {
FSINFO_DUMPER(MOUNT_INFO),
FSINFO_DUMPER(MOUNT_CHILDREN),
FSINFO_DUMPER(SERVER_ADDRESS),
+ FSINFO_DUMPER(SB_NOTIFICATIONS),
};
static void dump_fsinfo(enum fsinfo_attribute attr,
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox