lkml.org 
[lkml]   [2011]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] coredump: fix pipe coredump when core limit is 0
On 08/21, bookjovi@gmail.com wrote:
>
> From: Jovi Zhang <bookjovi@gmail.com>
>
> Regressing from 2.6.35

Hmm. Thanks Jovi.

> In pipe coredump case, normally core limits are irrelevant,
> since we're not writing to the file system, but core limit 0
> is a special value, kernel should skip the dump when limit is 0.

Hmm. probably yes... although I'd say I do not really know. iirc,
previously RLIMIT_CORE was simply ignored if ispipe. But then we
changed the rules many time.

Yes. See 725eae32df7754044809973034429a47e6035158. This is where
we changed the "limit == 0 && ispipe" behaviour.

> This error intruduced by commit c71354 in 2.6.35, that commit put
> core limit zero check into non-pipe code branch.
>
> commit c713541125002b8bc9e681af3b09118e771e2d8a
> Author: Oleg Nesterov <oleg@redhat.com>
> Date: Wed May 26 14:43:05 2010 -0700
>
> coredump: factor out the not-ispipe file checks

Cough. I don't think so ;)

Yes, that patch moves the check, but please note that before the patch
we did

if ((!ispipe) && (cprm.limit < binfmt->min_coredump))
goto fail;

so I do not think this patch can make any difference.

I think this was changed by 898b374af6f71041bd3bceebe257e564f3f1d458.

> For non-pipe case, limit 0 also means drop the coredump, so just put
> the zero limit check at do_coredump function begining.

Neil, what do you think? Should we change the code or the comment?


As for the patch, it is not exactly right in any case,

> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -2119,6 +2119,10 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
> if (!__get_dumpable(cprm.mm_flags))
> goto fail;
>
> + /* Core limit as 0 should skip the dump */
> + if (cprm.limit == 0)
> + goto fail;

Even if we do not dump, we should kill all tasks/threads which use
this ->mm. We shouldn't miss coredump_wait().

To clarify, I don't really know _why_, and probably it makes sense
to change this behaviour. But this needs a separate patch plus
discussion.

Oleg.


\
 
 \ /
  Last update: 2011-08-21 17:31    [W:0.044 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site