[Devel] Re: [PATCH 4/9][cr][v2]: Restore file_owner info

[Sukadev Bhattiprolu]

Oren Laadan [orenl at cs.columbia.edu] wrote:
| > +
| > +	rcu_read_lock();
| > +	pid = find_vpid(h->f_owner_pid);
| 
| What if this fails - the pid is invalid/non-existent ?

Good point. ->f_owner_pid can be 0 (in the normal case) and __fsetown()
below will set the owner to NULL pid. But if ->f_owner_pid is non-zero,
we should ensure we found a valid pid - added a check for this.

| 
| > +	/*
| > +	 * TODO: Do we need to force==1 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);
| > +	rcu_read_unlock();
| 
| I wonder if this would be a problem in terms of security on a
| non-container restart (e.g. not in a new pid-ns): one could set
| any pid as owner and any signal to be sent, and cause an arbitrary
| signal to be sent to an arbitrary process ?

Yes, Matt and Serge pointed it out and for now we need CAP_KILL
capability to restore an application that has file-leases.

Sukadev
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers