diff mbox

warning - cannot set dumpable: Invalid argument

Message ID 20060710210008.de1eb995.hilse@web.de
State New
Headers

Commit Message

Hans-Werner Hilse July 10, 2006, 7 p.m. UTC 
  Hi,

On Mon, 10 Jul 2006 20:20:53 +0200 Sebastian Kemper
<sebastian_ml@gmx.net> wrote:

> I installed kernel 2.6.16.24 lately and get this warning when starting
> vdr. The same should happen with 2.6.17.4 since both share the same
> Changelog:

(Part of?) the according patch is this:
http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.17.4.bz2;z=17

> So I was wondering if you could adapt vdr's dumpfile
> handling to the new situation and put the changes in vdr-1.4.2 in
> order to get rid of the warning, please. 

That would be getting rid of dump file "handling" at all, I think,
since PR_SET_DUMPABLE=1 is probably _not_ wanted since it reintroduces
the problems why =2 was made before. So basically this means: No core
dumps when s{u,g}id'ing.

A quick patch is appended.

-hwh

Comments

Pedja Vasiljevic July 11, 2006, 7:02 a.m. UTC | #1 
and how to install this patch?

On 7/10/06, Hans-Werner Hilse <hilse@web.de> wrote:
>
> Hi,
>
> On Mon, 10 Jul 2006 20:20:53 +0200 Sebastian Kemper
> <sebastian_ml@gmx.net> wrote:
>
> > I installed kernel 2.6.16.24 lately and get this warning when starting
> > vdr. The same should happen with 2.6.17.4 since both share the same
> > Changelog:
>
> (Part of?) the according patch is this:
>
> http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.17.4.bz2;z=17
>
> > So I was wondering if you could adapt vdr's dumpfile
> > handling to the new situation and put the changes in vdr-1.4.2 in
> > order to get rid of the warning, please.
>
> That would be getting rid of dump file "handling" at all, I think,
> since PR_SET_DUMPABLE=1 is probably _not_ wanted since it reintroduces
> the problems why =2 was made before. So basically this means: No core
> dumps when s{u,g}id'ing.
>
> A quick patch is appended.
>
> -hwh
>
>
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>
>
>
>
Hans-Werner Hilse July 11, 2006, 11:08 a.m. UTC | #2 
Hi,

On Tue, 11 Jul 2006 09:02:49 +0200 "Pedja Vasiljevic"
<pedja.vasiljevic@gmail.com> wrote:

> and how to install this patch?

$ cd vdr-1.4.1
$ patch < /whereever/youve/stored/it/PR_SET_DUMPABLE.patch

(of course, it's a _source_ patch, not for some binary...)

-hwh
Ville Skyttä July 14, 2006, 8:15 p.m. UTC | #3 
On Mon, 2006-07-10 at 21:00 +0200, Hans-Werner Hilse wrote:

> That would be getting rid of dump file "handling" at all, I think,
> since PR_SET_DUMPABLE=1 is probably _not_ wanted since it reintroduces
> the problems why =2 was made before. So basically this means: No core
> dumps when s{u,g}id'ing.

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.
Klaus Schmidinger July 15, 2006, 3:04 p.m. UTC | #4 
Hans-Werner Hilse wrote:
> Hi,
> 
> On Mon, 10 Jul 2006 20:20:53 +0200 Sebastian Kemper
> <sebastian_ml@gmx.net> wrote:
> 
>> I installed kernel 2.6.16.24 lately and get this warning when starting
>> vdr. The same should happen with 2.6.17.4 since both share the same
>> Changelog:
> 
> (Part of?) the according patch is this:
> http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.17.4.bz2;z=17

Is this the final "fix" or just a quick hack to provide an
immediate workaround? IMHO the actual problem should be fixed
instead of removing a parameter option.

Besides, I don't like the introduction of an extra VDR command line
option for this. I suggest we either wait until there is a real
fix in the kernel or, if no such fix comes and the '2' parameter
to prctl() is actually voided, we go back to '1' and let the user
control whether there should be a core dump via 'ulimit'.

Klaus
Hans-Werner Hilse July 15, 2006, 4:01 p.m. UTC | #5 
Hi,

On Sat, 15 Jul 2006 17:04:06 +0200 Klaus Schmidinger
<Klaus.Schmidinger@cadsoft.de> wrote:

> > (Part of?) the according patch is this:
> > http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.17.4.bz2;z=17
> 
> Is this the final "fix" or just a quick hack to provide an
> immediate workaround? IMHO the actual problem should be fixed
> instead of removing a parameter option.

Correct. I don't think that anyone has made any decision in this case
yet. I think a full revert of this feature would basically be a revert
of this:

http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d6e711448137ca3301512cec41a2c2ce852b3d0a

I'm not convinced that it would be a good idea to restore the
functionality of making core dumps when running as a non-priviledged
user. A possibility would be to force core dumps into a preconfigured
directory, changeable via a /proc entry.

> Besides, I don't like the introduction of an extra VDR command line
> option for this. I suggest we either wait until there is a real
> fix in the kernel or, if no such fix comes and the '2' parameter
> to prctl() is actually voided, we go back to '1' and let the user
> control whether there should be a core dump via 'ulimit'.

I can't find that scenario very attractive, but that's probably just
me. I don't see any good explanation of why on the one hand allow to
drop privileges and on the other hand open that restriction by making
the ability of doing core dumps the default setting. But I can
certainly live with that, it's just a few lines in the code and a patch
would be easy to maintain. And, after all, users who care for security
can just start vdr as a non-root user.

-hwh
Klaus Schmidinger July 23, 2006, 9:15 a.m. UTC | #6 
Hans-Werner Hilse wrote:
> Hi,
> 
> On Sat, 15 Jul 2006 17:04:06 +0200 Klaus Schmidinger
> <Klaus.Schmidinger@cadsoft.de> wrote:
> 
>>> (Part of?) the according patch is this:
>>> http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.17.4.bz2;z=17
>> Is this the final "fix" or just a quick hack to provide an
>> immediate workaround? IMHO the actual problem should be fixed
>> instead of removing a parameter option.
> 
> Correct. I don't think that anyone has made any decision in this case
> yet. I think a full revert of this feature would basically be a revert
> of this:
> 
> http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d6e711448137ca3301512cec41a2c2ce852b3d0a
> 
> I'm not convinced that it would be a good idea to restore the
> functionality of making core dumps when running as a non-priviledged
> user. A possibility would be to force core dumps into a preconfigured
> directory, changeable via a /proc entry.
> 
>> Besides, I don't like the introduction of an extra VDR command line
>> option for this. I suggest we either wait until there is a real
>> fix in the kernel or, if no such fix comes and the '2' parameter
>> to prctl() is actually voided, we go back to '1' and let the user
>> control whether there should be a core dump via 'ulimit'.
> 
> I can't find that scenario very attractive, but that's probably just
> me. I don't see any good explanation of why on the one hand allow to
> drop privileges and on the other hand open that restriction by making
> the ability of doing core dumps the default setting. But I can
> certainly live with that, it's just a few lines in the code and a patch
> would be easy to maintain. And, after all, users who care for security
> can just start vdr as a non-root user.

I guess I'll leave everything in VDR as it is right now, and will
see whether the kernel hackers implement an actual fix (not just a
feature-drop) some day.

Klaus
diff mbox

Patch

--- vdr-1.4.1.O/vdr.c	2006-06-04 11:04:47.000000000 +0200
+++ vdr-1.4.1/vdr.c	2006-07-10 20:58:14.619325064 +0200
@@ -102,10 +102,6 @@ 
         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) {
-        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;
 }