[RESEND PATCH 2/2] MIPS syscall auditing patches
From: Ralf Baechle r...@linux-mips.org
These are the userland patches to go along with the recently posted
kernel patches. Many of the comments posted along with those also apply
here, in particular wrt. to the ABI situation.
Signed-off-by: Ralf Baechle r...@linux-mips.org
--
mlau: Rediffed the patch against current audit-trunk. Build-tested
on little-endian MIPS32.
diff -Naurp trunk/lib/Makefile.am trunk-work/lib/Makefile.am
--- trunk/lib/Makefile.am 2014-03-28 14:38:29.831405944 +0100
+++ trunk-work/lib/Makefile.am 2014-03-28 15:52:02.891982735 +0100
@@ -39,8 +39,9 @@ nodist_libaudit_la_SOURCES = $(BUILT_SOU
BUILT_SOURCES = actiontabs.h errtabs.h fieldtabs.h flagtabs.h \
ftypetabs.h i386_tables.h ia64_tables.h machinetabs.h \
- msg_typetabs.h optabs.h ppc_tables.h s390_tables.h \
- s390x_tables.h x86_64_tables.h
+ msg_typetabs.h optabs.h mips_o32_tables.h mips_n32_tables.h \
+ mips_n64_tables.h ppc_tables.h s390_tables.h s390x_tables.h \
+ x86_64_tables.h
if USE_ALPHA
BUILT_SOURCES += alpha_tables.h
endif
@@ -53,7 +54,8 @@ endif
noinst_PROGRAMS = gen_actiontabs_h gen_errtabs_h gen_fieldtabs_h \
gen_flagtabs_h gen_ftypetabs_h gen_i386_tables_h \
gen_ia64_tables_h gen_machinetabs_h gen_msg_typetabs_h \
- gen_optabs_h gen_ppc_tables_h gen_s390_tables_h \
+ gen_optabs_h gen_mips_o32_tables_h gen_mips_n32_tables_h \
+ gen_mips_n64_tables_h gen_ppc_tables_h gen_s390_tables_h \
gen_s390x_tables_h gen_x86_64_tables_h
if USE_ALPHA
noinst_PROGRAMS += gen_alpha_tables_h
@@ -137,6 +139,21 @@ gen_optabs_h_CFLAGS = $(AM_CFLAGS) '-DTA
optabs.h: gen_optabs_h Makefile
./gen_optabs_h --i2s op $@
+gen_mips_o32_tables_h_SOURCES = gen_tables.c gen_tables.h mips_o32_table.h
+gen_mips_o32_tables_h_CFLAGS = $(AM_CFLAGS) '-DTABLE_H=mips_o32_table.h'
+mips_o32_tables.h: gen_mips_o32_tables_h Makefile
+ ./gen_mips_o32_tables_h --lowercase --i2s --s2i mips_o32_syscall $@
+
+gen_mips_n32_tables_h_SOURCES = gen_tables.c gen_tables.h mips_n32_table.h
+gen_mips_n32_tables_h_CFLAGS = $(AM_CFLAGS) '-DTABLE_H=mips_n32_table.h'
+mips_n32_tables.h: gen_mips_n32_tables_h Makefile
+ ./gen_mips_n32_tables_h --lowercase --i2s --s2i mips_n32_syscall $@
+
+gen_mips_n64_tables_h_SOURCES = gen_tables.c gen_tables.h mips_n64_table.h
+gen_mips_n64_tables_h_CFLAGS = $(AM_CFLAGS) '-DTABLE_H=mips_n64_table.h'
+mips_n64_tables.h: gen_mips_n64_tables_h Makefile
+ ./gen_mips_n64_tables_h --lowercase --i2s --s2i mips_n64_syscall $@
+
gen_ppc_tables_h_SOURCES = gen_tables.c gen_tables.h ppc_table.h
gen_ppc_tables_h_CFLAGS = $(AM_CFLAGS) '-DTABLE_H=ppc_table.h'
ppc_tables.h: gen_ppc_tables_h Makefile
diff -Naurp trunk/lib/libaudit.c trunk-work/lib/libaudit.c
--- trunk/lib/libaudit.c2014-03-28 14:38:29.699404905 +0100
+++ trunk-work/lib/libaudit.c 2014-03-28 16:22:15.141833267 +0100
@@ -1078,7 +1078,10 @@ int audit_determine_machine(const char *
} else if (strcasecmp(b32, arch) == 0) {
bits = ~__AUDIT_ARCH_64BIT;
machine = audit_detect_machine();
- }
+ } else if (strcasecmp(n32, arch) == 0) {
+ bits = __AUDIT_ARCH_64BIT;
+ machine = MACH_MIPS64_N32;
+ }
else
machine = audit_name_to_machine(arch);
@@ -1095,6 +1098,8 @@ int audit_determine_machine(const char *
machine = MACH_S390;
else if (bits == ~__AUDIT_ARCH_64BIT machine == MACH_AARCH64)
machine = MACH_ARM;
+ else if (bits == ~__AUDIT_ARCH_64BIT machine == MACH_MIPS64)
+ machine = MACH_MIPS;
/* Check for errors - return -6
* We don't allow 32 bit machines to specify 64 bit. */
@@ -1128,9 +1133,15 @@ int audit_determine_machine(const char *
return -6;
break;
#endif
- case MACH_86_64: /* fallthrough */
- case MACH_PPC64: /* fallthrough */
- case MACH_S390X: /* fallthrough */
+ case MACH_MIPS:
+ if (bits == __AUDIT_ARCH_64BIT)
+ return -6;
+ break;
+ case MACH_86_64:/* fallthrough */
+ case MACH_PPC64:/* fallthrough */
+ case MACH_S390X:/* fallthrough */
+ case MACH_MIPS64_N32: /* fallthrough */
+ case MACH_MIPS64: /* fallthrough */
break;
default:
return -6;
@@ -1479,10 +1490,18 @@ static int audit_name_to_gid(const char
int audit_detect_machine(void)
{
struct utsname uts;
- if (uname(uts) == 0)
-// strcpy(uts.machine, x86_64);
- return audit_name_to_machine(uts.machine);
- return -1;
+ int little_endian;
+
+ if (uname(uts) == -1)
+ return -1;
+
[RESEND PATCH 1/2] MIPS syscall auditing patches
From: Ralf Baechle r...@linux-mips.org
this is the first cut of the MIPS auditing patches. MIPS doesn't quite
fit into the existing pattern of other architectures and I'd appreciate
your comments and maybe even an Acked-by.
- MIPS syscalls return a success / error flag in register $7. If the
flag is set then the return value in $2 is a *positive* error value.
This means the existing AUDITSC_RESULT() macro does not work on
MIPS and thus ptrace.c defines it's own version MIPS_AUDITSC_RESULT().
- Linux on MIPS extends the traditional syscall table used by older UNIX
implementations. This is why 32-bit Linux syscalls are starting from
4000; the native 64-bit syscalls start from 5000 and the N32 compat ABI
from 6000. The existing syscall bitmap is only large enough for at most
2048 syscalls, so I had to increase AUDIT_BITMASK_SIZE to 256 which
provides enough space for 8192 syscalls. Because include/linux/audit.h
and AUDIT_BITMASK_SIZE are exported to userspace I've used an #ifdef
__mips__ for this.
- I've introduced a flag __AUDIT_ARCH_ALT to indicate an alternative ABI.
The name of the flag is intentionally very generic to make the name
hopefully fit other architectures' eventual need as well. On MIPS it
indicates the 3rd ABI known as N32.
- To make matters worse, most MIPS processors can be configured to be
big or little endian. Traditionally the the 64-bit little endian
configuration is named mips64el, so I've changed references to MIPSEL64
in audit.h to MIPS64EL.
- The code treats the little endian MIPS architecture as separate from
big endian. Combined with the 3 ABIs that's 6 combinations. I tried
to sort of follow the example set by ARM which explicitly lists the
(rare) big endian architecture variant - but it doesn't seem to very
useful so I wonder if this could be squashed to just the three ABIs
without consideration of endianess?
- Talking about flags; I've defined the the N32 architecture flags were defined
#define AUDIT_ARCH_MIPS64_N32 (EM_MIPS|__AUDIT_ARCH_ALT)
#define AUDIT_ARCH_MIPS64EL_N32 (EM_MIPS|__AUDIT_ARCH_ALT|__AUDIT_ARCH_LE
N32 is a 32-bit ABI but one that only runs on 64-bit processors as it
uses 64-bit registers for 64-bit integers. So I'm uncertain if the
__AUDIT_ARCH_64BIT flags should be set or not.
Thanks in advance,
Signed-off-by: Ralf Baechle r...@linux-mips.org
---
mlau: this is the patch Ralf sent in June 2011, I've just rediffed it against
latest linux (3.15-rc0).
arch/mips/Kconfig | 12 +
arch/mips/include/asm/abi.h | 1 +
arch/mips/include/uapi/asm/unistd.h | 18 +---
arch/mips/kernel/Makefile | 4 ++
arch/mips/kernel/audit-n32.c| 58 +++
arch/mips/kernel/audit-native.c | 92 +
arch/mips/kernel/audit-o32.c| 60
arch/mips/kernel/ptrace.c | 7 +++
arch/mips/kernel/signal.c | 20 +++-
arch/mips/kernel/signal32.c | 10 +++-
arch/mips/kernel/signal_n32.c | 10 +++-
include/uapi/linux/audit.h | 21 -
init/Kconfig| 3 +-
13 files changed, 305 insertions(+), 11 deletions(-)
create mode 100644 arch/mips/kernel/audit-n32.c
create mode 100644 arch/mips/kernel/audit-native.c
create mode 100644 arch/mips/kernel/audit-o32.c
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ecccd15..f1435b1 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -49,6 +49,7 @@ config MIPS
select CLONE_BACKWARDS
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_CC_STACKPROTECTOR
+ select AUDIT_ARCH
menu Machine selection
@@ -846,6 +847,15 @@ config FW_ARC
config ARCH_MAY_HAVE_PC_FDC
bool
+config AUDIT_ARCH
+ bool
+
+config AUDITSYSCALL_O32
+ bool
+
+config AUDITSYSCALL_N32
+ bool
+
config BOOT_RAW
bool
@@ -2516,6 +2526,7 @@ config SYSVIPC_COMPAT
config MIPS32_O32
bool Kernel support for o32 binaries
depends on MIPS32_COMPAT
+ select AUDITSYSCALL_O32 if AUDITSYSCALL
help
Select this option if you want to run o32 binaries. These are pure
32-bit binaries as used by the 32-bit Linux/MIPS port. Most of
@@ -2526,6 +2537,7 @@ config MIPS32_O32
config MIPS32_N32
bool Kernel support for n32 binaries
depends on MIPS32_COMPAT
+ select AUDITSYSCALL_N32 if AUDITSYSCALL
help
Select this option if you want to run n32 binaries. These are
64-bit binaries using 32-bit quantities for addressing and certain
diff --git a/arch/mips/include/asm/abi.h b/arch/mips/include/asm/abi.h
index 909bb69..7ae5eed 100644
--- a/arch/mips/include/asm/abi.h
+++ b/arch/mips/include/asm/abi.h
@@ -22,6 +22,7 @@ struct mips_abi {
sigset_t *set, siginfo_t *info);
const unsigned long
[RESEND PATCH 0/2] MIPS syscall auditing patches
From: Ralf Baechle r...@linux-mips.org Hello, This is a resend of the syscall auditing patches for MIPS, as sent by Ralf Baechle almost 3 years ago [1]. I've rediffed them against latest linux kernels and audit userland trunk. Here's what Ralf said then: This is the first cut of the MIPS auditing patches. MIPS doesn't quite fit into the existing pattern of other architectures and I'd appreciate your comments and maybe even an Acked-by for the kernel part so I can feed that upstream. Ralf Comments welcome! Thanks, Manuel Lauss [1] https://www.redhat.com/archives/linux-audit/2011-June/msg00027.html -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RESEND PATCH 1/2] MIPS syscall auditing patches
On 14/04/02, Manuel Lauss wrote: From: Ralf Baechle r...@linux-mips.org this is the first cut of the MIPS auditing patches. MIPS doesn't quite fit into the existing pattern of other architectures and I'd appreciate your comments and maybe even an Acked-by. - MIPS syscalls return a success / error flag in register $7. If the flag is set then the return value in $2 is a *positive* error value. This means the existing AUDITSC_RESULT() macro does not work on MIPS and thus ptrace.c defines it's own version MIPS_AUDITSC_RESULT(). - Linux on MIPS extends the traditional syscall table used by older UNIX implementations. This is why 32-bit Linux syscalls are starting from 4000; the native 64-bit syscalls start from 5000 and the N32 compat ABI from 6000. The existing syscall bitmap is only large enough for at most 2048 syscalls, so I had to increase AUDIT_BITMASK_SIZE to 256 which provides enough space for 8192 syscalls. Because include/linux/audit.h and AUDIT_BITMASK_SIZE are exported to userspace I've used an #ifdef __mips__ for this. Is this really necessary? I don't have any background on the choice of syscall numbers on MIPS. Instead of 4000, 5000, and 6000, could it not have used 500, 1000, 1500 as base numbers given that there are 350 syscalls? - I've introduced a flag __AUDIT_ARCH_ALT to indicate an alternative ABI. The name of the flag is intentionally very generic to make the name hopefully fit other architectures' eventual need as well. On MIPS it indicates the 3rd ABI known as N32. Is N32 sufficiently different from the concept of compat that that could not be used? Does the behaviour of 64-bit integers prevent this? - To make matters worse, most MIPS processors can be configured to be big or little endian. Traditionally the the 64-bit little endian configuration is named mips64el, so I've changed references to MIPSEL64 in audit.h to MIPS64EL. - The code treats the little endian MIPS architecture as separate from big endian. Combined with the 3 ABIs that's 6 combinations. I tried to sort of follow the example set by ARM which explicitly lists the (rare) big endian architecture variant - but it doesn't seem to very useful so I wonder if this could be squashed to just the three ABIs without consideration of endianess? In ARM's case, endian-ness doesn't affect the ABI, from what I understand. - Talking about flags; I've defined the the N32 architecture flags were defined #define AUDIT_ARCH_MIPS64_N32 (EM_MIPS|__AUDIT_ARCH_ALT) #define AUDIT_ARCH_MIPS64EL_N32 (EM_MIPS|__AUDIT_ARCH_ALT|__AUDIT_ARCH_LE N32 is a 32-bit ABI but one that only runs on 64-bit processors as it uses 64-bit registers for 64-bit integers. So I'm uncertain if the __AUDIT_ARCH_64BIT flags should be set or not. I would guess it should, but I am no expert. Thanks in advance, Signed-off-by: Ralf Baechle r...@linux-mips.org --- mlau: this is the patch Ralf sent in June 2011, I've just rediffed it against latest linux (3.15-rc0). arch/mips/Kconfig | 12 + arch/mips/include/asm/abi.h | 1 + arch/mips/include/uapi/asm/unistd.h | 18 +--- arch/mips/kernel/Makefile | 4 ++ arch/mips/kernel/audit-n32.c| 58 +++ arch/mips/kernel/audit-native.c | 92 + arch/mips/kernel/audit-o32.c| 60 arch/mips/kernel/ptrace.c | 7 +++ arch/mips/kernel/signal.c | 20 +++- arch/mips/kernel/signal32.c | 10 +++- arch/mips/kernel/signal_n32.c | 10 +++- include/uapi/linux/audit.h | 21 - init/Kconfig| 3 +- 13 files changed, 305 insertions(+), 11 deletions(-) create mode 100644 arch/mips/kernel/audit-n32.c create mode 100644 arch/mips/kernel/audit-native.c create mode 100644 arch/mips/kernel/audit-o32.c diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index ecccd15..f1435b1 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -49,6 +49,7 @@ config MIPS select CLONE_BACKWARDS select HAVE_DEBUG_STACKOVERFLOW select HAVE_CC_STACKPROTECTOR + select AUDIT_ARCH menu Machine selection @@ -846,6 +847,15 @@ config FW_ARC config ARCH_MAY_HAVE_PC_FDC bool +config AUDIT_ARCH + bool + +config AUDITSYSCALL_O32 + bool + +config AUDITSYSCALL_N32 + bool + config BOOT_RAW bool @@ -2516,6 +2526,7 @@ config SYSVIPC_COMPAT config MIPS32_O32 bool Kernel support for o32 binaries depends on MIPS32_COMPAT + select AUDITSYSCALL_O32 if AUDITSYSCALL help Select this option if you want to run o32 binaries. These are pure 32-bit binaries as used by the 32-bit Linux/MIPS port. Most of @@ -2526,6 +2537,7 @@ config MIPS32_O32 config
[PATCH] integrity: get comm using lock to avoid race in string printing
When task-comm is passed directly to audit_log_untrustedstring() without
getting a copy or using the task_lock, there is a race that could happen that
would output a NULL (\0) in the output string that would effectively truncate
the rest of the report text after the comm= field in the audit, losing fields.
Use get_task_comm() to get a copy while acquiring the task_lock to prevent
this and to prevent the result from being a mixture of old and new values of
comm.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
security/integrity/integrity_audit.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/security/integrity/integrity_audit.c
b/security/integrity/integrity_audit.c
index 85253b5..11706a2 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -33,6 +33,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
const char *cause, int result, int audit_info)
{
struct audit_buffer *ab;
+ char comm[sizeof(current-comm)];
if (!integrity_audit_info audit_info == 1) /* Skip info messages */
return;
@@ -49,7 +50,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
audit_log_format(ab, cause=);
audit_log_string(ab, cause);
audit_log_format(ab, comm=);
- audit_log_untrustedstring(ab, current-comm);
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
if (fname) {
audit_log_format(ab, name=);
audit_log_untrustedstring(ab, fname);
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
On Wed, 2014-04-02 at 12:19 -0400, Richard Guy Briggs wrote:
When task-comm is passed directly to audit_log_untrustedstring() without
getting a copy or using the task_lock, there is a race that could happen that
would output a NULL (\0) in the output string that would effectively truncate
the rest of the report text after the comm= field in the audit, losing fields.
Use get_task_comm() to get a copy while acquiring the task_lock to prevent
this and to prevent the result from being a mixture of old and new values of
comm.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
security/integrity/integrity_audit.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/security/integrity/integrity_audit.c
b/security/integrity/integrity_audit.c
index 85253b5..11706a2 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -33,6 +33,7 @@ void integrity_audit_msg(int audit_msgno, struct inode
*inode,
const char *cause, int result, int audit_info)
{
struct audit_buffer *ab;
+ char comm[sizeof(current-comm)];
if (!integrity_audit_info audit_info == 1) /* Skip info messages */
return;
@@ -49,7 +50,7 @@ void integrity_audit_msg(int audit_msgno, struct inode
*inode,
audit_log_format(ab, cause=);
audit_log_string(ab, cause);
audit_log_format(ab, comm=);
- audit_log_untrustedstring(ab, current-comm);
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
if (fname) {
audit_log_format(ab, name=);
audit_log_untrustedstring(ab, fname);
This change is already being upstreamed as commit 73a6b44 Integrity:
Pass commname via get_task_comm().
thanks,
Mimi
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
Hello Mimi, On Wednesday, April 02, 2014 01:39:47 PM Mimi Zohar wrote: This change is already being upstreamed as commit 73a6b44 Integrity: Pass commname via get_task_comm(). While I was looking at Richard's patch, I noticed a few places where cause and op are logged and the string isn't tied together with a _ or -. These are in ima/ima_appraise.c line 383, and ima/ima_policy.c lines 333, 657, and 683. Are these fixed upstream? Or should a patch be made? Thanks, -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
On Wed, 2014-04-02 at 14:00 -0400, Steve Grubb wrote: Hello Mimi, On Wednesday, April 02, 2014 01:39:47 PM Mimi Zohar wrote: This change is already being upstreamed as commit 73a6b44 Integrity: Pass commname via get_task_comm(). While I was looking at Richard's patch, I noticed a few places where cause and op are logged and the string isn't tied together with a _ or -. These are in ima/ima_appraise.c line 383, and ima/ima_policy.c lines 333, 657, and 683. Are these fixed upstream? Or should a patch be made? Nothing has changed in terms of 'cause' and 'op'. I would suggest making the changes in integrity_audit.c: integrity_audit_msg(). thanks, Mimi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
On Wed, 2014-04-02 at 14:12 -0400, Mimi Zohar wrote: On Wed, 2014-04-02 at 14:00 -0400, Steve Grubb wrote: Hello Mimi, On Wednesday, April 02, 2014 01:39:47 PM Mimi Zohar wrote: This change is already being upstreamed as commit 73a6b44 Integrity: Pass commname via get_task_comm(). While I was looking at Richard's patch, I noticed a few places where cause and op are logged and the string isn't tied together with a _ or -. These are in ima/ima_appraise.c line 383, and ima/ima_policy.c lines 333, 657, and 683. Are these fixed upstream? Or should a patch be made? Nothing has changed in terms of 'cause' and 'op'. I would suggest making the changes in integrity_audit.c: integrity_audit_msg(). The question is actually, do you know of anyone who is expecting the space, instead of a more 'audit standard' - or _ ? If not, we'll change it. If so, we'll discuss more :) -Eric -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
On Wed, 2014-04-02 at 14:18 -0400, Eric Paris wrote: On Wed, 2014-04-02 at 14:12 -0400, Mimi Zohar wrote: On Wed, 2014-04-02 at 14:00 -0400, Steve Grubb wrote: Hello Mimi, On Wednesday, April 02, 2014 01:39:47 PM Mimi Zohar wrote: This change is already being upstreamed as commit 73a6b44 Integrity: Pass commname via get_task_comm(). While I was looking at Richard's patch, I noticed a few places where cause and op are logged and the string isn't tied together with a _ or -. These are in ima/ima_appraise.c line 383, and ima/ima_policy.c lines 333, 657, and 683. Are these fixed upstream? Or should a patch be made? Nothing has changed in terms of 'cause' and 'op'. I would suggest making the changes in integrity_audit.c: integrity_audit_msg(). The question is actually, do you know of anyone who is expecting the space, instead of a more 'audit standard' - or _ ? If not, we'll change it. If so, we'll discuss more :) CC'ing linux-ima-user as well. Mimi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
oraphaned keywords in audit log text [was: Re: [PATCH] integrity: get comm using lock to avoid race in string] printing
On 14/04/02, Mimi Zohar wrote: On Wed, 2014-04-02 at 14:18 -0400, Eric Paris wrote: On Wed, 2014-04-02 at 14:12 -0400, Mimi Zohar wrote: On Wed, 2014-04-02 at 14:00 -0400, Steve Grubb wrote: Hello Mimi, On Wednesday, April 02, 2014 01:39:47 PM Mimi Zohar wrote: This change is already being upstreamed as commit 73a6b44 Integrity: Pass commname via get_task_comm(). While I was looking at Richard's patch, I noticed a few places where cause and op are logged and the string isn't tied together with a _ or -. These are in ima/ima_appraise.c line 383, and ima/ima_policy.c lines 333, 657, and 683. Are these fixed upstream? Or should a patch be made? Nothing has changed in terms of 'cause' and 'op'. I would suggest making the changes in integrity_audit.c: integrity_audit_msg(). That function could massage incoming text fields and convert spaces to hyphens or underscores, but I'd assume the right place to do it would be in the original text. If you suggest the former, it could just be done in audit_log_string(), but then grepping the source for error messages would not be nearly as useful. Is this what you were suggesting? The question is actually, do you know of anyone who is expecting the space, instead of a more 'audit standard' - or _ ? If not, we'll change it. If so, we'll discuss more :) CC'ing linux-ima-user as well. Thanks. Mimi - RGB -- Richard Guy Briggs rbri...@redhat.com Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] integrity: get comm using lock to avoid race in string printing
On 14/04/02, Mimi Zohar wrote:
On Wed, 2014-04-02 at 12:19 -0400, Richard Guy Briggs wrote:
When task-comm is passed directly to audit_log_untrustedstring() without
getting a copy or using the task_lock, there is a race that could happen
that
would output a NULL (\0) in the output string that would effectively
truncate
the rest of the report text after the comm= field in the audit, losing
fields.
Use get_task_comm() to get a copy while acquiring the task_lock to prevent
this and to prevent the result from being a mixture of old and new values of
comm.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
security/integrity/integrity_audit.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/security/integrity/integrity_audit.c
b/security/integrity/integrity_audit.c
index 85253b5..11706a2 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -33,6 +33,7 @@ void integrity_audit_msg(int audit_msgno, struct inode
*inode,
const char *cause, int result, int audit_info)
{
struct audit_buffer *ab;
+ char comm[sizeof(current-comm)];
if (!integrity_audit_info audit_info == 1) /* Skip info messages */
return;
@@ -49,7 +50,7 @@ void integrity_audit_msg(int audit_msgno, struct inode
*inode,
audit_log_format(ab, cause=);
audit_log_string(ab, cause);
audit_log_format(ab, comm=);
- audit_log_untrustedstring(ab, current-comm);
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
if (fname) {
audit_log_format(ab, name=);
audit_log_untrustedstring(ab, fname);
This change is already being upstreamed as commit 73a6b44 Integrity:
Pass commname via get_task_comm().
Excellent. Missed that. Thanks.
thanks,
Mimi
- RGB
--
Richard Guy Briggs rbri...@redhat.com
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
