Hi,
On Fri, 14 Jul 2006 23:15:38 +0300 Ville Skyttä <vskytta@gmail.com>
wrote:
> I'm not disputing this fix per se, but it is highly unfortunate, as
> VDR crashes are often hard to reproduce. Running as root is not
> acceptable in many setups, and even temporarily switching between
> root and the normal dedicated user results in annoyances such as file
> ownership issues in addition to the uncertainty whether the crash
> condition can be reproduced in the first place. Being able to run as
> non-root and have "secure" core dumps (which actually turned out to
> be not that secure) enabled and subject to ulimit -c just as usual
> was convenient.
>
> Would it be out of the question to add a command line option like
> --enable-insecure-core-dumps which when set and when run as non-root,
> would result in PR_SET_DUMPABLE=1, and otherwise no prctl() at all?
> This would get rid of some of the above difficulties.
I agree. Since these conditions can only arise when the process itself
has been started as root, it should be root's decision, too.
An updated patch is attached. It will probably need a bit of discussion
if the command switch I've introduced is properly named (userdump), if
the description fits and if the modus operandi is acceptable.
-hwh
@@ -82,7 +82,7 @@
static int Interrupted = 0;
-static bool SetUser(const char *UserName)
+static bool SetUser(const char *UserName, const bool UserDump)
{
if (UserName) {
struct passwd *user = getpwnam(UserName);
@@ -102,9 +102,8 @@
fprintf(stderr, "vdr: cannot set user id %u: %s\n", (unsigned int)user->pw_uid, strerror(errno));
return false;
}
- if (prctl(PR_SET_DUMPABLE, 2, 0, 0, 0) < 0) {
+ if (UserDump && prctl(PR_SET_DUMPABLE, 1, 0, 0, 0) < 0) {
fprintf(stderr, "vdr: warning - cannot set dumpable: %s\n", strerror(errno));
- // always non-fatal, and will not work with kernel < 2.6.13
}
}
return true;
@@ -174,6 +173,7 @@
bool StartedAsRoot = false;
const char *VdrUser = NULL;
+ bool UserDump = false;
int SVDRPport = DEFAULTSVDRPPORT;
const char *AudioCommand = NULL;
const char *ConfigDirectory = NULL;
@@ -228,6 +228,7 @@
{ "shutdown", required_argument, NULL, 's' },
{ "terminal", required_argument, NULL, 't' },
{ "user", required_argument, NULL, 'u' },
+ { "userdump", no_argument, NULL, 'u' | 0x100 },
{ "version", no_argument, NULL, 'V' },
{ "vfat", no_argument, NULL, 'v' | 0x100 },
{ "video", required_argument, NULL, 'v' },
@@ -325,6 +326,9 @@
case 'u': if (*optarg)
VdrUser = optarg;
break;
+ case 'u' | 0x100:
+ UserDump = true;
+ break;
case 'V': DisplayVersion = true;
break;
case 'v' | 0x100:
@@ -354,7 +358,7 @@
if (strcmp(VdrUser, "root")) {
if (!SetKeepCaps(true))
return 2;
- if (!SetUser(VdrUser))
+ if (!SetUser(VdrUser,UserDump))
return 2;
if (!SetKeepCaps(false))
return 2;
@@ -408,6 +412,7 @@
" -t TTY, --terminal=TTY controlling tty\n"
" -u USER, --user=USER run as user USER; only applicable if started as\n"
" root\n"
+ " --userdump allow coredumps when -u is given (debugging)\n"
" -v DIR, --video=DIR use DIR as video directory (default: %s)\n"
" -V, --version print version information and exit\n"
" --vfat encode special characters in recording names to\n"