Merge landlock_find_rule() into landlock_unmask_layers() so rule pointers stay inside the domain implementation while unmask checking gets the matched rule it needs for the check_rule tracepoint. landlock_unmask_layers() now takes a landlock_id and the domain instead of a rule pointer. A rename or link evaluates the same dentry against both renamed parents, so this path now looks the rule up once per parent; collapsing that back to a single lookup is left to a follow-up.
Emit, via the per-type wrappers unmask_layers_fs() and unmask_layers_net(), the rights each matching rule grants at every domain layer. The events carry this as a dynamic per-layer array (up to LANDLOCK_MAX_NUM_LAYERS entries) reserved from the trace ring buffer, not the caller's stack, and rendered symbolically per layer. A WARN_ON_ONCE() in __trace_landlock_fill_layers() flags a rule whose layer levels fall outside the domain range or are unsorted, a cannot-happen case; the zero-filled slots keep the rendered output and the array bounds safe regardless. Setting allowed_parent2 to true for non-dom-check requests when get_inode_id() returns false preserves the pre-refactoring behavior: a negative dentry (no backing inode) has no matching rule, so the access is allowed at this path component. Before the refactoring, landlock_unmask_layers() with a NULL rule produced this result as a side effect; now the caller must set it explicitly. Name the trace-only check_rule fields so each printk label equals its ring-buffer field name and works directly as an ftrace filter: the request field is labelled access_request= and the per-layer array is named grants. Values audit also logs keep audit's label (domain=, ruleset=) so a single filter works across trace and audit. Cc: Günther Noack <[email protected]> Cc: Justin Suess <[email protected]> Cc: Masami Hiramatsu <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Steven Rostedt <[email protected]> Cc: Tingmao Wang <[email protected]> Signed-off-by: Mickaël Salaün <[email protected]> --- Changes since v2: https://patch.msgid.link/[email protected] - Order the check_rule tracepoint arguments with the common part (domain, rule) before the event-specific access_request and object. - Reformatted TP_STRUCT__entry and TP_fast_assign to kernel tracing convention (requested by Steven Rostedt). - Render the request access right as symbolic names with __print_flags(), shared with the audit blocker names. - Rename the per-layer field label allowed= to grants= and render it symbolically via __print_landlock_layers(), intersected with the request to show the granted requested rights per layer (was a raw hex value). - Rename struct layer_access_masks to the base's struct layer_masks. - Rename the check_rule printk label request= to access_request= and the per-layer ring-buffer field layers to grants, so each trace-only field and its printk label share one name (usable directly as an ftrace filter); audit-shared labels (domain=, ruleset=) are unchanged. Changes since v1: https://patch.msgid.link/[email protected] - Merged find-rule consolidation (v1 2/5) into this patch. - Added check_rule_net tracepoint for network rules. - Added get_inode_id() helper with rcu_access_pointer(). - Added allowed_parent2 behavioral fix. --- include/trace/events/landlock.h | 203 ++++++++++++++++++++++++++++++++ security/landlock/domain.c | 32 +++-- security/landlock/domain.h | 10 +- security/landlock/fs.c | 149 +++++++++++++++-------- security/landlock/net.c | 21 +++- 5 files changed, 350 insertions(+), 65 deletions(-) diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h index 7f221d8fff38..c693248afe23 100644 --- a/include/trace/events/landlock.h +++ b/include/trace/events/landlock.h @@ -11,10 +11,13 @@ #define _TRACE_LANDLOCK_H #include <linux/landlock.h> +#include <linux/trace_seq.h> #include <linux/tracepoint.h> +struct dentry; struct landlock_domain; struct landlock_hierarchy; +struct landlock_rule; struct landlock_ruleset; struct path; @@ -74,7 +77,110 @@ struct path; * (e.g. network ports are __u64 in host endianness, like * landlock_net_port_attr.port). Per-event details, such as where a value * is byte-swapped, live in the field's own kdoc. + * + * Rule-check fields + * ~~~~~~~~~~~~~~~~~ + * + * The check_rule events fire during an access check, once per matching + * rule, before the final allow-or-deny verdict. They share domain (the + * enforcing domain being evaluated), access_request (the access mask being + * checked), and rule (the matching rule, with per-layer access masks). + */ + +#ifdef CREATE_TRACE_POINTS + +/* + * Fills the dense per-domain-layer array layers (one access mask per layer, + * indexed by level - 1) from rule's sparse layer stack, keeping only the + * requested rights (access_request). Layers with no matching rule entry get + * a zero mask. Shared by the check_rule_fs and check_rule_net events. + * + * rule->layers is sorted by ascending level, with levels in the domain's + * [1, num_layers] range (see landlock_merge_ruleset()), so every entry maps + * to a slot. A leftover entry would be a malformed rule; the zero-filled + * slots keep the output and the array bounds safe regardless. */ +static inline void +__trace_landlock_fill_layers(access_mask_t *const layers, + const size_t num_layers, + const struct landlock_rule *const rule, + const access_mask_t access_request) +{ + size_t i = 0; + + for (size_t level = 1; level <= num_layers; level++) { + access_mask_t grants = 0; + + if (i < rule->num_layers && level == rule->layers[i].level) { + grants = rule->layers[i].access & access_request; + i++; + } + layers[level - 1] = grants; + } + + /* A leftover entry means an out-of-range or unsorted rule level. */ + WARN_ON_ONCE(i < rule->num_layers); +} + +/* + * Renders the dense per-domain-layer access array as symbolic flag names for + * the grants field: layers wrapped in "{}", flags within a layer joined by + * "|", layers separated by ",", an empty layer rendered as nothing. + * Open-codes the flag walk because trace_print_flags_seq() NUL-terminates per + * call and so cannot be chained into a single field. The shared names table + * covers every access right, so masked bits are always named. Returns the + * trace_seq position like __print_flags(). + */ +static inline const char * +__trace_landlock_print_layers(struct trace_seq *p, + const access_mask_t *const layers, + const size_t num_layers, + const struct trace_print_flags *const names, + const size_t names_size) +{ + const char *const ret = trace_seq_buffer_ptr(p); + + trace_seq_putc(p, '{'); + for (size_t i = 0; i < num_layers; i++) { + access_mask_t mask = layers[i]; + bool first = true; + + if (i) + trace_seq_putc(p, ','); + for (size_t j = 0; mask && j < names_size; j++) { + if ((mask & names[j].mask) != names[j].mask) + continue; + if (!first) + trace_seq_putc(p, '|'); + trace_seq_puts(p, names[j].name); + mask &= ~names[j].mask; + first = false; + } + } + trace_seq_putc(p, '}'); + trace_seq_putc(p, 0); + return ret; +} + +#endif /* CREATE_TRACE_POINTS */ + +/* + * Prints a per-layer access mask array (the dynamic array @array) as symbolic + * flag names using the shared @flag_names list (a _LANDLOCK_*_NAMES macro). + * Stays outside CREATE_TRACE_POINTS: TP_printk is expanded in the print-output + * pass where that macro is undefined. + */ +#define __print_landlock_layers(array, flag_names...) \ + ({ \ + static const struct trace_print_flags __layer_names[] = { \ + flag_names \ + }; \ + __trace_landlock_print_layers( \ + p, __get_dynamic_array(array), \ + __get_dynamic_array_len(array) / \ + sizeof(access_mask_t), \ + __layer_names, ARRAY_SIZE(__layer_names)); \ + }) /** * landlock_create_ruleset - New ruleset created @@ -374,6 +480,103 @@ TRACE_EVENT(landlock_free_domain, __entry->domain_id, __entry->denials) ); +/** + * landlock_check_rule_fs - Filesystem rule evaluated during access check + * + * @domain: Enforcing domain (never NULL). + * @rule: Matching rule with per-layer access masks (never NULL). + * @access_request: Access mask evaluated against the rule (the domain's + * handled mask during rename/link double-checks). + * @dentry: Filesystem dentry being checked (never NULL). + * + * Emitted for each rule that matches during a filesystem access check. + * The grants array shows the requested rights the rule grants at each + * domain layer. See Documentation/trace/events-landlock.rst for how to + * interpret it. + */ +TRACE_EVENT(landlock_check_rule_fs, + + TP_PROTO(const struct landlock_domain *domain, + const struct landlock_rule *rule, + access_mask_t access_request, const struct dentry *dentry), + + TP_ARGS(domain, rule, access_request, dentry), + + TP_STRUCT__entry( + __field( __u64, domain_id ) + __field( access_mask_t, access_request ) + __field( dev_t, dev ) + __field( ino_t, ino ) + __dynamic_array(access_mask_t, grants, + domain->num_layers) + ), + + TP_fast_assign( + __entry->domain_id = domain->hierarchy->id; + __entry->access_request = access_request; + __entry->dev = dentry->d_sb->s_dev; + __entry->ino = d_backing_inode(dentry)->i_ino; + + __trace_landlock_fill_layers(__get_dynamic_array(grants), + __get_dynamic_array_len(grants) / + sizeof(access_mask_t), + rule, access_request); + ), + + TP_printk("domain=%llx access_request=%s dev=%u:%u ino=%lu grants=%s", + __entry->domain_id, + __print_flags(__entry->access_request, "|", _LANDLOCK_ACCESS_FS_NAMES), + MAJOR(__entry->dev), MINOR(__entry->dev), __entry->ino, + __print_landlock_layers(grants, _LANDLOCK_ACCESS_FS_NAMES)) +); + +/** + * landlock_check_rule_net - Network port rule evaluated during access check + * + * @domain: Enforcing domain (never NULL). + * @rule: Matching rule with per-layer access masks (never NULL). + * @access_request: Access mask being requested. + * @port: Network port being checked (host endianness). + * + * Emitted for each rule that matches during a network access check. The + * grants array shows the requested rights the rule grants at each domain + * layer. See Documentation/trace/events-landlock.rst for how to + * interpret it. + */ +TRACE_EVENT(landlock_check_rule_net, + + TP_PROTO(const struct landlock_domain *domain, + const struct landlock_rule *rule, + access_mask_t access_request, __u64 port), + + TP_ARGS(domain, rule, access_request, port), + + TP_STRUCT__entry( + __field( __u64, domain_id ) + __field( access_mask_t, access_request ) + __field( __u64, port ) + __dynamic_array(access_mask_t, grants, + domain->num_layers) + ), + + TP_fast_assign( + __entry->domain_id = domain->hierarchy->id; + __entry->access_request = access_request; + __entry->port = port; + + __trace_landlock_fill_layers(__get_dynamic_array(grants), + __get_dynamic_array_len(grants) / + sizeof(access_mask_t), + rule, access_request); + ), + + TP_printk("domain=%llx access_request=%s port=%llu grants=%s", + __entry->domain_id, + __print_flags(__entry->access_request, "|", _LANDLOCK_ACCESS_NET_NAMES), + __entry->port, + __print_landlock_layers(grants, _LANDLOCK_ACCESS_NET_NAMES)) +); + #undef _LANDLOCK_NAME_ENTRY #endif /* _TRACE_LANDLOCK_H */ diff --git a/security/landlock/domain.c b/security/landlock/domain.c index 082c4da68536..c20020024de4 100644 --- a/security/landlock/domain.c +++ b/security/landlock/domain.c @@ -97,9 +97,9 @@ void landlock_put_domain_deferred(struct landlock_domain *const domain) } /* The returned access has the same lifetime as the domain. */ -const struct landlock_rule * -landlock_find_rule(const struct landlock_domain *const domain, - const struct landlock_id id) +static const struct landlock_rule * +find_rule(const struct landlock_domain *const domain, + const struct landlock_id id) { const struct rb_root *root; const struct rb_node *node; @@ -126,26 +126,38 @@ landlock_find_rule(const struct landlock_domain *const domain, /** * landlock_unmask_layers - Remove the access rights in @masks which are - * granted in @rule + * granted by a matching rule * - * Updates the set of (per-layer) unfulfilled access rights @masks so that all - * the access rights granted in @rule are removed from it (because they are now - * fulfilled). + * Looks up the rule matching @id in @domain, then updates the set of + * (per-layer) unfulfilled access rights @masks so that all the access rights + * granted by that rule are removed (because they are now fulfilled). * - * @rule: A rule that grants a set of access rights for each layer. + * @domain: The Landlock domain to search for a matching rule. + * @id: Identifier for the rule target (e.g. inode, port). * @masks: A matrix of unfulfilled access rights for each layer. + * @matched_rule: Optional output for the matched rule (for tracing); set to + * the matching rule when non-NULL, unchanged otherwise. * * Return: True if the request is allowed (i.e. the access rights granted all * remaining unfulfilled access rights and masks has no leftover set bits). */ -bool landlock_unmask_layers(const struct landlock_rule *const rule, - struct layer_masks *masks) +bool landlock_unmask_layers(const struct landlock_domain *const domain, + const struct landlock_id id, + struct layer_masks *masks, + const struct landlock_rule **matched_rule) { + const struct landlock_rule *rule; + if (!masks) return true; + + rule = find_rule(domain, id); if (!rule) return false; + if (matched_rule) + *matched_rule = rule; + /* * An access is granted if, for each policy layer, at least one rule * encountered on the pathwalk grants the requested access, regardless diff --git a/security/landlock/domain.h b/security/landlock/domain.h index 8351e22016fe..5baa4a73b446 100644 --- a/security/landlock/domain.h +++ b/security/landlock/domain.h @@ -305,12 +305,10 @@ struct landlock_domain * landlock_merge_ruleset(struct landlock_domain *const parent, struct landlock_ruleset *const ruleset); -const struct landlock_rule * -landlock_find_rule(const struct landlock_domain *const domain, - const struct landlock_id id); - -bool landlock_unmask_layers(const struct landlock_rule *const rule, - struct layer_masks *masks); +bool landlock_unmask_layers(const struct landlock_domain *const domain, + const struct landlock_id id, + struct layer_masks *masks, + const struct landlock_rule **matched_rule); access_mask_t landlock_init_layer_masks(const struct landlock_domain *const domain, diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 48744a21d0a3..fe028aac26ae 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -376,31 +376,55 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset, /* Access-control management */ -/* - * The lifetime of the returned rule is tied to @domain. +/** + * get_inode_id - Look up the Landlock object for a dentry + * @dentry: The dentry to look up. + * @id: Filled with the inode's Landlock object pointer on success. + * + * Extracts the Landlock object pointer from @dentry's inode security blob and + * stores it in @id for use as a rule-tree lookup key. * - * Returns NULL if no rule is found or if @dentry is negative. + * When this returns false (negative dentry or no Landlock object), no rule can + * match this inode, so landlock_unmask_layers() need not be called. Callers + * that gate landlock_unmask_layers() on this function must handle the NULL + * masks case independently, since the !masks-returns-true early-return in + * landlock_unmask_layers() will not be reached. See the allowed_parent2 + * initialization in is_access_to_paths_allowed(). + * + * Return: True if a Landlock object exists for @dentry, false otherwise. */ -static const struct landlock_rule * -find_rule(const struct landlock_domain *const domain, - const struct dentry *const dentry) +static bool get_inode_id(const struct dentry *const dentry, + struct landlock_id *id) { - const struct landlock_rule *rule; - const struct inode *inode; - struct landlock_id id = { - .type = LANDLOCK_KEY_INODE, - }; - /* Ignores nonexistent leafs. */ if (d_is_negative(dentry)) - return NULL; + return false; - inode = d_backing_inode(dentry); - rcu_read_lock(); - id.key.object = rcu_dereference(landlock_inode(inode)->object); - rule = landlock_find_rule(domain, id); - rcu_read_unlock(); - return rule; + /* + * rcu_access_pointer() is sufficient: the pointer is used only as a + * numeric comparison key for rule lookup, not dereferenced. The object + * cannot be freed while the domain exists because the domain's rule + * tree holds its own reference to it. + */ + id->key.object = rcu_access_pointer( + landlock_inode(d_backing_inode(dentry))->object); + return !!id->key.object; +} + +static bool unmask_layers_fs(const struct landlock_domain *const domain, + const struct landlock_id id, + const access_mask_t access_request, + struct layer_masks *masks, + const struct dentry *const dentry) +{ + const struct landlock_rule *rule = NULL; + bool ret; + + ret = landlock_unmask_layers(domain, id, masks, &rule); + if (rule) + trace_landlock_check_rule_fs(domain, rule, access_request, + dentry); + return ret; } /* @@ -781,6 +805,9 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain, bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check, child1_is_directory = true, child2_is_directory = true; struct path walker_path; + struct landlock_id id = { + .type = LANDLOCK_KEY_INODE, + }; access_mask_t access_masked_parent1, access_masked_parent2; struct layer_masks _layer_masks_child1, _layer_masks_child2; struct layer_masks *layer_masks_child1 = NULL, @@ -820,28 +847,46 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain, /* For a simple request, only check for requested accesses. */ access_masked_parent1 = access_request_parent1; access_masked_parent2 = access_request_parent2; + /* + * Simple requests have no parent2 to check, so parent2 is + * trivially allowed. This must be set explicitly because the + * get_inode_id() gate in the pathwalk loop may prevent + * landlock_unmask_layers() from being called (which would + * otherwise return true for NULL masks as a side effect). + */ + allowed_parent2 = true; is_dom_check = false; } if (unlikely(dentry_child1)) { - /* - * Get the layer masks for the child dentries for use by domain - * check later. - */ - if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS, - &_layer_masks_child1, - LANDLOCK_KEY_INODE)) - landlock_unmask_layers(find_rule(domain, dentry_child1), - &_layer_masks_child1); + struct landlock_id id = { + .type = LANDLOCK_KEY_INODE, + }; + access_mask_t handled; + + handled = landlock_init_layer_masks(domain, + LANDLOCK_MASK_ACCESS_FS, + &_layer_masks_child1, + LANDLOCK_KEY_INODE); + if (handled && get_inode_id(dentry_child1, &id)) + unmask_layers_fs(domain, id, handled, + &_layer_masks_child1, dentry_child1); layer_masks_child1 = &_layer_masks_child1; child1_is_directory = d_is_dir(dentry_child1); } if (unlikely(dentry_child2)) { - if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS, - &_layer_masks_child2, - LANDLOCK_KEY_INODE)) - landlock_unmask_layers(find_rule(domain, dentry_child2), - &_layer_masks_child2); + struct landlock_id id = { + .type = LANDLOCK_KEY_INODE, + }; + access_mask_t handled; + + handled = landlock_init_layer_masks(domain, + LANDLOCK_MASK_ACCESS_FS, + &_layer_masks_child2, + LANDLOCK_KEY_INODE); + if (handled && get_inode_id(dentry_child2, &id)) + unmask_layers_fs(domain, id, handled, + &_layer_masks_child2, dentry_child2); layer_masks_child2 = &_layer_masks_child2; child2_is_directory = d_is_dir(dentry_child2); } @@ -853,8 +898,6 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain, * restriction. */ while (true) { - const struct landlock_rule *rule; - /* * If at least all accesses allowed on the destination are * already allowed on the source, respectively if there is at @@ -895,13 +938,20 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain, break; } - rule = find_rule(domain, walker_path.dentry); - allowed_parent1 = - allowed_parent1 || - landlock_unmask_layers(rule, layer_masks_parent1); - allowed_parent2 = - allowed_parent2 || - landlock_unmask_layers(rule, layer_masks_parent2); + if (get_inode_id(walker_path.dentry, &id)) { + allowed_parent1 = + allowed_parent1 || + unmask_layers_fs(domain, id, + access_masked_parent1, + layer_masks_parent1, + walker_path.dentry); + allowed_parent2 = + allowed_parent2 || + unmask_layers_fs(domain, id, + access_masked_parent2, + layer_masks_parent2, + walker_path.dentry); + } /* Stops when a rule from each layer grants access. */ if (allowed_parent1 && allowed_parent2) @@ -1064,23 +1114,30 @@ static bool collect_domain_accesses(const struct landlock_domain *const domain, struct layer_masks *layer_masks_dom) { bool ret = false; + access_mask_t access_masked_dom; if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom)) return true; if (is_nouser_or_private(dir)) return true; - if (!landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS, - layer_masks_dom, LANDLOCK_KEY_INODE)) + access_masked_dom = + landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS, + layer_masks_dom, LANDLOCK_KEY_INODE); + if (!access_masked_dom) return true; dget(dir); while (true) { struct dentry *parent_dentry; + struct landlock_id id = { + .type = LANDLOCK_KEY_INODE, + }; /* Gets all layers allowing all domain accesses. */ - if (landlock_unmask_layers(find_rule(domain, dir), - layer_masks_dom)) { + if (get_inode_id(dir, &id) && + unmask_layers_fs(domain, id, access_masked_dom, + layer_masks_dom, dir)) { /* * Stops when all handled accesses are allowed by at * least one rule in each layer. diff --git a/security/landlock/net.c b/security/landlock/net.c index ead97fcfdcff..8f2aaac54b33 100644 --- a/security/landlock/net.c +++ b/security/landlock/net.c @@ -53,6 +53,22 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset, return err; } +static bool unmask_layers_net(const struct landlock_domain *const domain, + const struct landlock_id id, + struct layer_masks *masks, + access_mask_t access_request) +{ + const struct landlock_rule *rule = NULL; + bool ret; + + ret = landlock_unmask_layers(domain, id, masks, &rule); + if (rule) + trace_landlock_check_rule_net( + domain, rule, access_request, + ntohs((__force __be16)id.key.data)); + return ret; +} + static int current_check_access_socket(struct socket *const sock, struct sockaddr *const address, const int addrlen, @@ -62,7 +78,6 @@ static int current_check_access_socket(struct socket *const sock, unsigned short sock_family; __be16 port; struct layer_masks layer_masks = {}; - const struct landlock_rule *rule; struct landlock_id id = { .type = LANDLOCK_KEY_NET_PORT, }; @@ -248,14 +263,14 @@ static int current_check_access_socket(struct socket *const sock, id.key.data = (__force uintptr_t)port; BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data)); - rule = landlock_find_rule(subject->domain, id); access_request = landlock_init_layer_masks(subject->domain, access_request, &layer_masks, LANDLOCK_KEY_NET_PORT); if (!access_request) return 0; - if (landlock_unmask_layers(rule, &layer_masks)) + if (unmask_layers_net(subject->domain, id, &layer_masks, + access_request)) return 0; audit_net.family = address->sa_family; -- 2.54.0
