On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote: > Restore the file-owner information for each 'struct file'. This is > essentially is like a new fcntl(F_SETOWN) and fcntl(F_SETSIG) calls, > except that the pid, uid, euid and signum values are read from the > checkpoint image. > > Changelog[v4]: > - [Oren Laadan]: Sanitize the pid-type field read from checkpoint image. > Changelog[v3]: > - [Oren Laadan]: Ensure find_vpid() found a valid pid before > making it the file owner. > Changelog[v2]: > - [Matt Helsley, Serge Hallyn]: Don't trust uids in checkpoint image. > (added CAP_KILL check) > - Check that signal number read from the checkpoint image is valid. > (not sure it is required, since its an incomplete check for tampering) > > Signed-off-by: Sukadev Bhattiprolu <[email protected]> > --- > fs/checkpoint.c | 78 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 78 insertions(+), 0 deletions(-) > > diff --git a/fs/checkpoint.c b/fs/checkpoint.c > index ce1b4af..be9d39a 100644 > --- a/fs/checkpoint.c > +++ b/fs/checkpoint.c > @@ -618,6 +618,80 @@ static int attach_file(struct file *file) > return fd; > } > > +static int restore_file_owner(struct ckpt_ctx *ctx, struct ckpt_hdr_file *h, > + struct file *file) > +{ > + int ret; > + struct pid *pid; > + uid_t uid, euid; > + > + uid = h->f_owner_uid; > + euid = h->f_owner_euid; > + > + ckpt_debug("restore_file_owner(): uid %u, euid %u, pid %d, type %d\n", > + uid, euid, h->f_owner_pid, h->f_owner_pid_type); > + /* > + * We can't trust the uids in the checkpoint image and normally need > + * CAP_KILL. But if the uids match our ids, should be fine since we > + * have access to the file. > + * > + * TODO: Move this check to __f_setown() ? > + */ > + ret = -EACCES; > + if (!capable(CAP_KILL) && > + (uid != current_uid() || euid != current_euid())) { > + ckpt_err(ctx, ret, "image uids [%d, %d] don't match current " > + "process uids [%d, %d] and no CAP_KILL\n", > + uid, euid, current_uid(), current_euid()); > + return ret; > + } > + > + ret = -EINVAL; > + if (!valid_signal(h->f_owner_signum)) { > + ckpt_err(ctx, ret, "Invalid signum %d\n", h->f_owner_signum); > + return ret; > + } > + file->f_owner.signum = h->f_owner_signum; > + > + switch(h->f_owner_pid_type) { > + case PIDTYPE_PID: > + case PIDTYPE_PGID: > + case PIDTYPE_SID: > + break; > + default: > + ckpt_err(ctx, ret, "Invalid pid-type %d\n", > + h->f_owner_pid_type); > + return ret; > + > + } > + > + rcu_read_lock(); > + > + /* > + * If file had a non-NULL owner and we can't find the owner after > + * restart, return error. > + */ > + pid = find_vpid(h->f_owner_pid); > + if (h->f_owner_pid && !pid) > + ret = -ESRCH; > + else { > + /* > + * TODO: Do we need 'force' to be 1 here or can it be 0 ? > + * 'force' is used to modify the owner, if one is > + * already set. Can it be set when we restart an > + * application ? > + */ > + ret = __f_setown(file, pid, h->f_owner_pid_type, uid, euid, 1);
The owner should not be set by now. So I suggest removing the comment and bailing if already set. Oren. > + } > + > + rcu_read_unlock(); > + > + if (ret < 0) > + ckpt_err(ctx, ret, "__fsetown_uid() failed\n"); > + > + return ret; > +} > + > #define CKPT_SETFL_MASK \ > (O_APPEND | O_NONBLOCK | O_NDELAY | FASYNC | O_DIRECT | O_NOATIME) > > @@ -651,6 +725,10 @@ int restore_file_common(struct ckpt_ctx *ctx, struct > file *file, > if (ret < 0) > return ret; > > + ret = restore_file_owner(ctx, h, file); > + if (ret < 0) > + return ret; > + > /* > * Normally f_mode is set by open, and modified only via > * fcntl(), so its value now should match that at checkpoint. _______________________________________________ Containers mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Devel mailing list [email protected] https://openvz.org/mailman/listinfo/devel
