From 022732e3d846e197539712e51ecada90ded0572a Mon Sep 17 00:00:00 2001 From: Chris Riches Date: Wed, 18 Oct 2023 09:23:51 +0000 Subject: audit: Send netlink ACK before setting connection in auditd_set When auditd_set sets the auditd_conn pointer, audit messages can immediately be put on the socket by other kernel threads. If the backlog is large or the rate is high, this can immediately fill the socket buffer. If the audit daemon requested an ACK for this operation, a full socket buffer causes the ACK to get dropped, also setting ENOBUFS on the socket. To avoid this race and ensure ACKs get through, fast-track the ACK in this specific case to ensure it is sent before auditd_conn is set. Signed-off-by: Chris Riches [PM: fix some tab vs space damage] Signed-off-by: Paul Moore --- kernel/audit.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 16205dd29843..9c8e5f732c4c 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -487,15 +487,19 @@ static void auditd_conn_free(struct rcu_head *rcu) * @pid: auditd PID * @portid: auditd netlink portid * @net: auditd network namespace pointer + * @skb: the netlink command from the audit daemon + * @ack: netlink ack flag, cleared if ack'd here * * Description: * This function will obtain and drop network namespace references as * necessary. Returns zero on success, negative values on failure. */ -static int auditd_set(struct pid *pid, u32 portid, struct net *net) +static int auditd_set(struct pid *pid, u32 portid, struct net *net, + struct sk_buff *skb, bool *ack) { unsigned long flags; struct auditd_connection *ac_old, *ac_new; + struct nlmsghdr *nlh; if (!pid || !net) return -EINVAL; @@ -507,6 +511,13 @@ static int auditd_set(struct pid *pid, u32 portid, struct net *net) ac_new->portid = portid; ac_new->net = get_net(net); + /* send the ack now to avoid a race with the queue backlog */ + if (*ack) { + nlh = nlmsg_hdr(skb); + netlink_ack(skb, nlh, 0, NULL); + *ack = false; + } + spin_lock_irqsave(&auditd_conn_lock, flags); ac_old = rcu_dereference_protected(auditd_conn, lockdep_is_held(&auditd_conn_lock)); @@ -1200,7 +1211,8 @@ static int audit_replace(struct pid *pid) return auditd_send_unicast_skb(skb); } -static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) +static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh, + bool *ack) { u32 seq; void *data; @@ -1293,7 +1305,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) /* register a new auditd connection */ err = auditd_set(req_pid, NETLINK_CB(skb).portid, - sock_net(NETLINK_CB(skb).sk)); + sock_net(NETLINK_CB(skb).sk), + skb, ack); if (audit_enabled != AUDIT_OFF) audit_log_config_change("audit_pid", new_pid, @@ -1538,9 +1551,10 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) * Parse the provided skb and deal with any messages that may be present, * malformed skbs are discarded. */ -static void audit_receive(struct sk_buff *skb) +static void audit_receive(struct sk_buff *skb) { struct nlmsghdr *nlh; + bool ack; /* * len MUST be signed for nlmsg_next to be able to dec it below 0 * if the nlmsg_len was not aligned @@ -1553,9 +1567,12 @@ static void audit_receive(struct sk_buff *skb) audit_ctl_lock(); while (nlmsg_ok(nlh, len)) { - err = audit_receive_msg(skb, nlh); - /* if err or if this message says it wants a response */ - if (err || (nlh->nlmsg_flags & NLM_F_ACK)) + ack = nlh->nlmsg_flags & NLM_F_ACK; + err = audit_receive_msg(skb, nlh, &ack); + + /* send an ack if the user asked for one and audit_receive_msg + * didn't already do it, or if there was an error. */ + if (ack || err) netlink_ack(skb, nlh, err, NULL); nlh = nlmsg_next(nlh, &len); -- cgit v1.2.3 From 4e8714b76613e6284b263274d6dddcfac24be262 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 15 Nov 2023 12:09:17 -0500 Subject: MAINTAINERS: update the audit entry Bring the audit subsystem entry up to date with the following changes: * Add our patchwork link. I'm not sure this is of much use for anyone but the maintainer, but there is a provision for including it here so we might as well include it. * Add a bug report URI. I suspect most everyone knows to send mail to the mailing list if they hit a bug, but let's make it official. * Add a link to the audit tree process/management documentation. While the doc exists both in the canonical kernel.org location and the GitHub mirror, provide a link to the mirror as GitHub does a better job rendering the Markdown. * Update the source tree's git URI to use https. * Aside from changes to the audit code itself, we also would like to be notified when the audit call sites are changed so we are adding an audit_XXX(...) regex to try and catch all of the callers. Signed-off-by: Paul Moore --- MAINTAINERS | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 97f51d5ec1cf..bb967a494400 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3350,13 +3350,17 @@ M: Eric Paris L: audit@vger.kernel.org S: Supported W: https://github.com/linux-audit -T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git +Q: https://patchwork.kernel.org/project/audit/list +B: mailto:audit@vger.kernel.org +P: https://github.com/linux-audit/audit-kernel/blob/main/README.md +T: git https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git F: include/asm-generic/audit_*.h F: include/linux/audit.h F: include/linux/audit_arch.h F: include/uapi/linux/audit.h F: kernel/audit* F: lib/*audit.c +K: \baudit_[a-z_0-9]\+\b AUXILIARY BUS DRIVER M: Greg Kroah-Hartman -- cgit v1.2.3