linux-kernel - Re: [RFC v4][PATCH 2/9] General infrastructure for checkpoint restart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Sep 2008 02:44:36 -0400
From:	Oren Laadan <orenl@...columbia.edu>
To:	MinChan Kim <minchan.kim@...il.com>
CC:	dave@...ux.vnet.ibm.com, arnd@...db.de, jeremy@...p.org,
	linux-kernel@...r.kernel.org, containers@...ts.linux-foundation.org
Subject: Re: [RFC v4][PATCH 2/9] General infrastructure for checkpoint restart



MinChan Kim wrote:
> Hi, Oren.
> 
> On Thu, Sep 11, 2008 at 3:36 AM, Oren Laadan <orenl@...columbia.edu> wrote:
>>
>> MinChan Kim wrote:
>>> On Tue, Sep 9, 2008 at 4:42 PM, Oren Laadan <orenl@...columbia.edu> wrote:
>> [...]
>>
>>>> +struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags)
>>>> +{
>>>> +       struct cr_ctx *ctx;
>>>> +
>>>> +       ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>>>> +       if (!ctx)
>>>> +               return ERR_PTR(-ENOMEM);
>>>> +
>>>> +       ctx->file = fget(fd);
>>>> +       if (!ctx->file) {
>>>> +               cr_ctx_free(ctx);
>>>> +               return ERR_PTR(-EBADF);
>>>> +       }
>>>> +       get_file(ctx->file);
>>> Why do you need get_file?
>>> You already called fget.
>>> Am I missing something ?
>> This was meant for when we will restart multiple processes, each would
>> have access to the checkpoint-context, such that the checkpoint-context
>> may outlives the task that created it and initiated the restart. Thus
>> the file-pointer will need to stay around longer than that task.
> 
> OK. Thanks for your explanation.
> You should have inserted above annotation.
> 
>> Of course, restart of multiple processes _can_ be coded such that this
>> first task will always terminate last - either after restart completes
>> successfully, or after all the other tasks aborted and won't use the
>> checkpoint-context anymore.
>>
>> Because that code is not part of the this patch-set, I considered it
>> safer to grab a reference of the file pointer, making it less likely
>> that we forget about it later.
> 
> What do you mean by that ? Isn't it a your part of your code?
> When the last checkpoint-context is ended, who free file ?
> I mean how it match pair(fget/fput and get_file) ?

I meant that future code would make that clear, but that creates more
confusion than not.

Instead, I'll leave that to the future and for now just clean the code
as you suggested.

Oren.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/