Re: [PATCH] audit: always check the netlink payload length in audit_receive_msg()

On Mon, Feb 24, 2020 at 5:53 PM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > This patch ensures that we always check the netlink payload length > in audit_receive_msg() before we take any action on the payload > itself. > > Cc: stable(a)vger.kernel.org > Reported-by: syzbot+399c44bf1f43b8747403(a)syzkaller.appspotmail.com > Reported-by: syzbot+e4b12d8d202701f08b6d(a)syzkaller.appspotmail.com > Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; > --- > kernel/audit.c | 43 +++++++++++++++++++++++-------------------- > 1 file changed, 23 insertions(+), 20 deletions(-) ... > diff --git a/kernel/audit.c b/kernel/audit.c > index 17b0d523afb3..6e8b176bdb68 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1334,26 +1339,24 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) > break; > } > audit_log_user_recv_msg(&ab, msg_type); > - if (msg_type != AUDIT_USER_TTY) > + if (msg_type != AUDIT_USER_TTY) { > + /* ensure NULL termination */ > + str[data_len - 1] = '\0'; > audit_log_format(ab, " msg='%.*s'", > AUDIT_MESSAGE_TEXT_MAX, > - (char *)data); > - else { > - int size; > - > + str); > + } else { > audit_log_format(ab, " data="); > - size = nlmsg_len(nlh); > - if (size > 0 && > - ((unsigned char *)data)[size - 1] == '\0') > - size--; > - audit_log_n_untrustedstring(ab, data, size); > + if (data_len > 0 && str[data_len - 1] == '\0') > + data_len--; > + audit_log_n_untrustedstring(ab, data, data_len); ^^^^ ... and it looks like I didn't properly refresh my patch before sending, the second arg in the line above is "str" not "data".

paul moore