From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 79283C433F5 for ; Tue, 3 May 2022 16:08:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239165AbiECQLf (ORCPT ); Tue, 3 May 2022 12:11:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52252 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239024AbiECQLb (ORCPT ); Tue, 3 May 2022 12:11:31 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF9452B24C for ; Tue, 3 May 2022 09:07:57 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id j15so24070136wrb.2 for ; Tue, 03 May 2022 09:07:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=h9qkGkLhyYVuP5NA+EyqoC9Fx6dLw0IZC0J4czMcE3s=; b=Q3kJ8dp+glqL6hrMCvTstBZwmmJPz/CaYGwV/d2xk6xJBAeFfuS9shb832MfGT6Fuq vl08vQTfs4UHQQYYc2UqZf11rgvLU2YvesaMJS5RhCFvNtiW/pwTuiUcPd8Peko6upbq bHB3lt95E7lnHd4R9BBM5jhWGwQ/hlmhJJyqo5l7uNoZQ+IFVLKmDYf5sL+TXof0K0zA JQG/IG5XHWr2bomNbJy5JvGUXC9dBXVhCJnoETdpR6S/zBanM7P/cWxwq81aSLeonEYq /e3VP5qHvXG7fDN198EXXI2Gl3Pfu9oKifURmXEGQHlInf/4en50cV/cARyn9fdyaYs0 0PKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=h9qkGkLhyYVuP5NA+EyqoC9Fx6dLw0IZC0J4czMcE3s=; b=ZtIMUPOf92Jp1p4nDIK7wtKiukDnMf5h3VJbykCJudYqD/X2q6T1pxP20i7l0zlU1b ln/547G2g17b+WysWXPt9Ckci7OTaHvoMMFlSKhGtg8Hy5b7GQRfaK7DkVeVJd1bezja eeQP1uupinld8my+ZH0cCYPmND8dTktA3dEF106aXQpsfjxYHIQftTyHIAu+fKM5Q7SX u3oBbDD0xmrA5uwKpog40y2R4TIkZ3KIt/KJkYPzDdjWLveKzvVn2RNhWsy3uYFCblWq Os1eFmolOATAqK0SFRVT1F5gMeC2wIJzGSDoctjTYJ/Bj0GW6wr2DgC530ifzrrMvcRq kKXA== X-Gm-Message-State: AOAM533m6c0b6cF0Sx3YBtmeCs0a0hDZCDmGs/ISFYZGYVN04syWWdXr xG2LMU8XB4FrToHBmYRcW0DY2qFJDEErrbOV2uCD X-Google-Smtp-Source: ABdhPJybVow9Pu2mDV8hRgGZZILEuK6b9eT5oZLMJZzrGb8IbCTHvuar2Z5/Ftf+TBLoXXGDuX6+xSauHFiv5ehLslM= X-Received: by 2002:a05:6000:80e:b0:20c:5b45:a700 with SMTP id bt14-20020a056000080e00b0020c5b45a700mr9669529wrb.662.1651594076393; Tue, 03 May 2022 09:07:56 -0700 (PDT) MIME-Version: 1.0 References: <20220503090212.1322050-1-svens@linux.ibm.com> In-Reply-To: <20220503090212.1322050-1-svens@linux.ibm.com> From: Paul Moore Date: Tue, 3 May 2022 12:07:45 -0400 Message-ID: Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing To: Sven Schnelle Cc: Eric Paris , linux-audit@redhat.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 3, 2022 at 5:02 AM Sven Schnelle wrote: > > socketcall auditing misses the call argument: > > type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c > > which renders socketcall auditing (almost) useless. Add the call > argument so it is possible to decode the actual syscall from the > audit log: > > type=SOCKETCALL msg=audit: call=1 nargs=3 a0=10 a1=3 a2=c > > Signed-off-by: Sven Schnelle > --- > include/linux/audit.h | 10 +++++----- > kernel/audit.h | 1 + > kernel/auditsc.c | 6 ++++-- > net/compat.c | 2 +- > net/socket.c | 2 +- > 5 files changed, 12 insertions(+), 9 deletions(-) Hi Sven, Thanks for catching this, my only guess is that the original code assumed that a0 held the socket call number. In addition to the kernel test robot errors that need fixing, I've made some comments inline with the patch below ... > diff --git a/kernel/audit.h b/kernel/audit.h > index 58b66543b4d5..34e53b6f0ebb 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -153,6 +153,7 @@ struct audit_context { > int type; > union { > struct { > + int call; > int nargs; > long args[6]; Not your code, but while you are making changes, perhaps make @args[6] and unsigned long to match the network stack's code. > } socketcall; > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index ea2ee1181921..c856893041c9 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1399,8 +1399,9 @@ static void show_special(struct audit_context *context, int *call_panic) > switch (context->type) { > case AUDIT_SOCKETCALL: { > int nargs = context->socketcall.nargs; > + int call = context->socketcall.call; > > - audit_log_format(ab, "nargs=%d", nargs); > + audit_log_format(ab, "call=%d nargs=%d", call, nargs); > for (i = 0; i < nargs; i++) > audit_log_format(ab, " a%d=%lx", i, > context->socketcall.args[i]); The approach we take when adding new fields to existing audit records is to add them to the end of the record. Using your example in the patch description, we would want to see the following record format for SOCKETCALL: type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c call=1 > @@ -2684,13 +2685,14 @@ void __audit_bprm(struct linux_binprm *bprm) > * @args: args array > * > */ > -int __audit_socketcall(int nargs, unsigned long *args) > +int __audit_socketcall(int call, int nargs, unsigned long *args) > { > struct audit_context *context = audit_context(); > > if (nargs <= 0 || nargs > AUDITSC_ARGS || !args) > return -EINVAL; > context->type = AUDIT_SOCKETCALL; > + context->socketcall.call = call; > context->socketcall.nargs = nargs; > memcpy(context->socketcall.args, args, nargs * sizeof(unsigned long)); > return 0; > diff --git a/net/compat.c b/net/compat.c > index 210fc3b4d0d8..0df955019ecc 100644 > --- a/net/compat.c > +++ b/net/compat.c > @@ -437,7 +437,7 @@ COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args) > if (copy_from_user(a, args, len)) > return -EFAULT; > > - ret = audit_socketcall_compat(len / sizeof(a[0]), a); > + ret = audit_socketcall_compat(call, len / sizeof(a[0]), a); See my note below for the non-compat version of socketcall(2). > diff --git a/net/socket.c b/net/socket.c > index 6887840682bb..ff71f28c96f7 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -2921,7 +2921,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args) > if (copy_from_user(a, args, len)) > return -EFAULT; > > - err = audit_socketcall(nargs[call] / sizeof(unsigned long), a); > + err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a); I recognize that this isn't your code, but I think it might be better to cleanup the arg count calculation passed as the second parameter. Something like this not only looks cleaner, but it should be a bit more robust against other kernel changes: err = audit_socketcall(call, len / AL(1), a); ... it may also help resolve some of the kernel test robot errors you are seeing. -- paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2C298C433EF for ; Tue, 3 May 2022 16:08:21 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-393-eRifEj9SMDe7W1MSKTUbbQ-1; Tue, 03 May 2022 12:08:03 -0400 X-MC-Unique: eRifEj9SMDe7W1MSKTUbbQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EC216BE0013; Tue, 3 May 2022 16:08:00 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (unknown [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD0671121320; Tue, 3 May 2022 16:08:00 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 6451E1947049; Tue, 3 May 2022 16:08:00 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id B68FB1947043 for ; Tue, 3 May 2022 16:07:59 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id A9A491537E17; Tue, 3 May 2022 16:07:59 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast01.extmail.prod.ext.rdu2.redhat.com [10.11.55.17]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A5EAB1537E10 for ; Tue, 3 May 2022 16:07:59 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8EFB986B8BA for ; Tue, 3 May 2022 16:07:59 +0000 (UTC) Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-464-7oK7DwqvOVi0Tt9FO5IC1g-1; Tue, 03 May 2022 12:07:57 -0400 X-MC-Unique: 7oK7DwqvOVi0Tt9FO5IC1g-1 Received: by mail-wr1-f46.google.com with SMTP id e2so24025192wrh.7 for ; Tue, 03 May 2022 09:07:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=h9qkGkLhyYVuP5NA+EyqoC9Fx6dLw0IZC0J4czMcE3s=; b=RaRJwTzXEhhD5UE3H2h499eHsAaCo1J4mpyNqnYHVrceisiZH6H4K5KNDtvpeUJOLs pYi2QB1VZQnRoXC9Sa5+qLppKd7IOp4cmqMckQoX6M5Z3BwvATNVNjlit02kpYNi5XJK yowkMUQ8Y45Yj6Lvw0B4H+HkryY3FJPJqXhR8RhEqayJojyt+bMzRBxHI14R1sVp2FuT gO/EoqJ8Ce5dO9gfZkj4lb8d9v7QnZeKPe2NV4VEl1jOoI0kAdq1eT7ZRTRNrsvkND6d uFriqna0dWx4tpYK0ujroHaPmLycRvf1uIS+EwygExztGIbM0ObvmPwW4D+4+trmhwNt N8rA== X-Gm-Message-State: AOAM530VUhcfGAEWB19RjA9FQiLkWFTKtE4wpsUa21bxA+iWXD2FiUEg i2lIDLSancaFQyrcmTjjZk2uPDCPQLDuQ3rWT2we X-Google-Smtp-Source: ABdhPJybVow9Pu2mDV8hRgGZZILEuK6b9eT5oZLMJZzrGb8IbCTHvuar2Z5/Ftf+TBLoXXGDuX6+xSauHFiv5ehLslM= X-Received: by 2002:a05:6000:80e:b0:20c:5b45:a700 with SMTP id bt14-20020a056000080e00b0020c5b45a700mr9669529wrb.662.1651594076393; Tue, 03 May 2022 09:07:56 -0700 (PDT) MIME-Version: 1.0 References: <20220503090212.1322050-1-svens@linux.ibm.com> In-Reply-To: <20220503090212.1322050-1-svens@linux.ibm.com> From: Paul Moore Date: Tue, 3 May 2022 12:07:45 -0400 Message-ID: Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing To: Sven Schnelle X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 X-BeenThere: linux-audit@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Audit Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, Eric Paris Errors-To: linux-audit-bounces@redhat.com Sender: "Linux-audit" X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=linux-audit-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Tue, May 3, 2022 at 5:02 AM Sven Schnelle wrote: > > socketcall auditing misses the call argument: > > type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c > > which renders socketcall auditing (almost) useless. Add the call > argument so it is possible to decode the actual syscall from the > audit log: > > type=SOCKETCALL msg=audit: call=1 nargs=3 a0=10 a1=3 a2=c > > Signed-off-by: Sven Schnelle > --- > include/linux/audit.h | 10 +++++----- > kernel/audit.h | 1 + > kernel/auditsc.c | 6 ++++-- > net/compat.c | 2 +- > net/socket.c | 2 +- > 5 files changed, 12 insertions(+), 9 deletions(-) Hi Sven, Thanks for catching this, my only guess is that the original code assumed that a0 held the socket call number. In addition to the kernel test robot errors that need fixing, I've made some comments inline with the patch below ... > diff --git a/kernel/audit.h b/kernel/audit.h > index 58b66543b4d5..34e53b6f0ebb 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -153,6 +153,7 @@ struct audit_context { > int type; > union { > struct { > + int call; > int nargs; > long args[6]; Not your code, but while you are making changes, perhaps make @args[6] and unsigned long to match the network stack's code. > } socketcall; > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index ea2ee1181921..c856893041c9 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1399,8 +1399,9 @@ static void show_special(struct audit_context *context, int *call_panic) > switch (context->type) { > case AUDIT_SOCKETCALL: { > int nargs = context->socketcall.nargs; > + int call = context->socketcall.call; > > - audit_log_format(ab, "nargs=%d", nargs); > + audit_log_format(ab, "call=%d nargs=%d", call, nargs); > for (i = 0; i < nargs; i++) > audit_log_format(ab, " a%d=%lx", i, > context->socketcall.args[i]); The approach we take when adding new fields to existing audit records is to add them to the end of the record. Using your example in the patch description, we would want to see the following record format for SOCKETCALL: type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c call=1 > @@ -2684,13 +2685,14 @@ void __audit_bprm(struct linux_binprm *bprm) > * @args: args array > * > */ > -int __audit_socketcall(int nargs, unsigned long *args) > +int __audit_socketcall(int call, int nargs, unsigned long *args) > { > struct audit_context *context = audit_context(); > > if (nargs <= 0 || nargs > AUDITSC_ARGS || !args) > return -EINVAL; > context->type = AUDIT_SOCKETCALL; > + context->socketcall.call = call; > context->socketcall.nargs = nargs; > memcpy(context->socketcall.args, args, nargs * sizeof(unsigned long)); > return 0; > diff --git a/net/compat.c b/net/compat.c > index 210fc3b4d0d8..0df955019ecc 100644 > --- a/net/compat.c > +++ b/net/compat.c > @@ -437,7 +437,7 @@ COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args) > if (copy_from_user(a, args, len)) > return -EFAULT; > > - ret = audit_socketcall_compat(len / sizeof(a[0]), a); > + ret = audit_socketcall_compat(call, len / sizeof(a[0]), a); See my note below for the non-compat version of socketcall(2). > diff --git a/net/socket.c b/net/socket.c > index 6887840682bb..ff71f28c96f7 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -2921,7 +2921,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args) > if (copy_from_user(a, args, len)) > return -EFAULT; > > - err = audit_socketcall(nargs[call] / sizeof(unsigned long), a); > + err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a); I recognize that this isn't your code, but I think it might be better to cleanup the arg count calculation passed as the second parameter. Something like this not only looks cleaner, but it should be a bit more robust against other kernel changes: err = audit_socketcall(call, len / AL(1), a); ... it may also help resolve some of the kernel test robot errors you are seeing. -- paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit