graphene icon indicating copy to clipboard operation
graphene copied to clipboard

[LibOS] FS-specific checkpoint callback must not leak checkpoint data

Open dimakuv opened this issue 6 years ago • 1 comments

PR #596 fixes the bug of the FS-specific checkpoint callback being called only the first time a process forks (so, when a process forks a second child, the second child may get a stale checkpoint / not perform checkpoint-specific code). The fix is to unconditionally call the FS-specific checkpoint callback.

Currently, it doesn't represent a problem. The only FS which has a checkpoint callback is Chroot, and it does a simple assignment of an already existing checkpoint stored somewhere in Chroot-specific memory:

static int chroot_checkpoint (void ** checkpoint, void * mount_data) {
    ...
    *checkpoint = mount_data;
     ...
}

However, there can be a memory leak if the checkpoint callback allocates data, e.g.:

my_fs_checkpoint(void ** checkpoint, void * mount_data) {  *checkpoint = malloc(1024) }

We can either mandate that checkpoint callbacks must never allocate memory or introduce a generic mechanism to clean the previous checkpoint during BEGIN_CP_FUNC(mount).

dimakuv avatar Jun 07 '19 21:06 dimakuv

How about

  • stop NULL'fy by the caller
  • on second (or later) call of checkpoint callback, it's up to the callback to keep the previous one or free the old one/allocate new.

yamahata avatar Jun 07 '19 21:06 yamahata