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]
Message-ID: <48C8BE54.3050909@cs.columbia.edu>
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ