Permalink
Browse files

Fix invocation to syslog

  • Loading branch information...
1 parent 1ae6c34 commit f49164bf7658855390b2c19574b170f8728a99ff @migueldeicaza migueldeicaza committed Jul 25, 2011
Showing with 1 addition and 1 deletion.
  1. +1 −1 support/syslog.c
View
2 support/syslog.c
@@ -34,7 +34,7 @@ Mono_Posix_Syscall_closelog (void)
int
Mono_Posix_Syscall_syslog (int priority, const char* message)
{
- syslog (priority, message);
+ syslog (priority, "%s", message);
return 0;
}

4 comments on commit f49164b

@jonpryor
Mono Project member

I'm way late to seeing this, but...what prompted this change?

The message parameter is validated in managed code:
https://github.com/mono/mono/blob/master/mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs#L3200
https://github.com/mono/mono/blob/master/mcs/class/Mono.Posix/Mono.Unix/UnixMarshal.cs#L366

This change means that '%m' isn't useful in format strings to Syscall.syslog(), as it'll be escaped instead of "passed through" unchanged.

@meebey

It was changed because of this build error (with hardening options):

../doltcompile gcc -DHAVE_CONFIG_H -I. -I..  -I../eglib/src -I../eglib/src -I.. -D_FORTIFY_SOURCE=2 -DGC_LINUX_THREADS -D_GNU_SOURCE -D_REENTRANT -DUSE_MMAP -DUSE_MUNMAP  -D__default_codegen__ -DUSE_COMPILER_TLS  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -fno-strict-aliasing -Wdeclaration-after-statement -Wno-unused-but-set-variable -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -Wno-switch-enum -mno-tls-direct-seg-refs -Werror-implicit-function-declaration -MT syslog.lo -MD -MP -MF .deps/syslog.Tpo -c -o syslog.lo syslog.c
syslog.c: In function 'Mono_Posix_Syscall_syslog':
syslog.c:37:2: error: format not a string literal and no format arguments [-Werror=format-security]
syslog.c: At top level:
syslog.c:43:1: warning: no previous prototype for 'Mono_Posix_Syscall_syslog2' [-Wmissing-prototypes]
cc1: some warnings being treated as errors
make[3]: *** [syslog.lo] Error 1
@jonpryor
Mono Project member

Is there no other way to remove that error? Again, this means that %m can't be used in strings from managed code for some Syscall.syslog() calls (see below). ( syslog(3): "`%m' is replaced by the current error message.")

I suppose we could just accept that limitation, document it, and document a workaround (e.g. "%m is not supported. To print the current error messages, use Stdlib.strerror(Errno).").

However, the Syscall.syslog(SyslogFacility, SyslogLevel, string, object[]) overload WILL permit use of %m (and all other printf(3) format strings), as it goes ~directly to Mono_Posix_Syscall_syslog2(), which has no such error/limitation.

The result is that this doesn't work as expected:

Syscall.syslog(SyslogFacility.LOG_USER, SyslogLevel.LOG_ERR, "Some error occurred: %m");
// prints: Some error occurred: %m

while this does:

Syscall.syslog(SyslogFacility.LOG_USER, SyslogLevel.LOG_ERR, "Some error occurred for user: %s: %m", "username");
// prints: Some error occurred for user: username: [insert strerror() output here]

This has the potential to make refactoring "weird": add an argument to Syscall.syslog(), and you can use %m. Have no arguments, and you can't. Wat?

Does gcc have a pragma we can use to disable that error within Mono_Posix_Syscall_syslog?

@meebey

This is currently a show stopper for Mono 3.0 deb packages (as the build fails and Debian forces the hardening flags). Looks like this could be the solution: http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

Please sign in to comment.