From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934501AbbI1Qof (ORCPT ); Mon, 28 Sep 2015 12:44:35 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:46844 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934013AbbI1Qod (ORCPT ); Mon, 28 Sep 2015 12:44:33 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Oleg Nesterov Cc: Konstantin Khlebnikov , linux-api@vger.kernel.org, containers@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Roman Gushchin , Serge Hallyn , Chen Fan , Andrew Morton , Linus Torvalds , =?utf-8?Q?St=C3=A9phane?= Graber References: <20150925135246.27620.97496.stgit@buzz> <20150925175654.GA12504@redhat.com> Date: Mon, 28 Sep 2015 11:37:08 -0500 In-Reply-To: <20150925175654.GA12504@redhat.com> (Oleg Nesterov's message of "Fri, 25 Sep 2015 19:56:54 +0200") Message-ID: <871tdi8pqj.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1+khmBbLUrKfunRL7TMb+oc7+B02q3CHhU= X-SA-Exim-Connect-IP: 67.3.201.231 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 1.0 XMGappySubj_02 Gappier still * 0.5 XMGappySubj_01 Very gappy subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.4890] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject * 0.0 T_TooManySym_03 6+ unique symbols in subject X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Oleg Nesterov X-Spam-Relay-Country: X-Spam-Timing: total 382 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 3.0 (0.8%), b_tie_ro: 2.1 (0.6%), parse: 1.34 (0.4%), extract_message_metadata: 5 (1.3%), get_uri_detail_list: 3.0 (0.8%), tests_pri_-1000: 6 (1.5%), tests_pri_-950: 1.80 (0.5%), tests_pri_-900: 1.54 (0.4%), tests_pri_-400: 41 (10.7%), check_bayes: 39 (10.3%), b_tokenize: 12 (3.2%), b_tok_get_all: 10 (2.5%), b_comp_prob: 3.0 (0.8%), b_tok_touch_all: 2.6 (0.7%), b_finish: 0.78 (0.2%), tests_pri_0: 303 (79.5%), tests_pri_500: 4.3 (1.1%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH 0/1] ns: introduce proc_get_ns_by_fd() X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Oleg Nesterov writes: > On 09/25, Konstantin Khlebnikov wrote: >> >> +struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref) >> { >> - struct file *file; >> + struct ns_common *ns; >> + struct fd f; >> >> - file = fget(fd); >> - if (!file) >> + f = fdget(fd); >> + if (!f.file) >> return ERR_PTR(-EBADF); >> >> - if (file->f_op != &ns_file_operations) >> + if (f.file->f_op != &ns_file_operations) >> + goto out_invalid; >> + >> + ns = get_proc_ns(file_inode(f.file)); >> + if (nstype && (ns->ops->type != nstype)) >> goto out_invalid; >> >> - return file; >> + *fd_ref = f; >> + return ns; >> >> out_invalid: >> - fput(file); >> + fdput(f); >> return ERR_PTR(-EINVAL); >> } > > Well yes, fdget() makes sense but this is minor. > > Honestly, I do not really like the new helper... I understand this > is subjective, so I won't insist. But how about 1/1? We do not need > fd/file at all. With this patch your sys_getvpid() can just use > proc_get_ns_by_fd(fd, CLONE_NEWPID) and put_pid_ns(). > > Eric, what do you think? At some level I don't care this is not exposed to userspace. However since we are going several rounds with this. Of the existing uses several of them sleep, which unfortunately means we can not use rcu locking for everything. The network namespace ones wind up taking a reference to struct net because the have the legacy pid case to deal with. Which makes we can not use fdget for all callers either. For this translate_pid rcu locking is sufficient, rcu locking is easy and doing any more than rcu locking just seems silly. So let me respectfully suggest. struct ns_common *ns_by_fd_rcu(int fd, int type) { struct files_struct *files = current->files; struct file *file; struct ns_common *ns; void *ret; file = fcheck_files(files, fd); if (!file) return ERR_PTR(-EBADF); if (file->f_mode & FMODE_PATH) return ERR_PTR(-EINVAL); if (file->f_op != &ns_file_operations) return ERR_PTR(-EINVAL); ns = get_proc_ns(file_inode(file)); if (ns->ops->type != type) return ERR_PTR(-EINVAL); return ns; } struct pid_namespace *pidns_by_fd_rcu(int fd) { struct ns_common *ns = ns_by_fd_rcu(fd, CLONE_NEWPID); if (IS_ERR(ns)) return ERR_CAST(ns); return container_of(ns, struct pid_namespace, ns); } SYSCALL_DEFINE3(translate_pid, pid_t, pid_nr, int, sourcefd, int, targetfd) { struct pid_namespace *source, *target; struct pid *pid; pid_t result; rcu_read_lock(); if (sourcefd >= 0) source = pidns_by_fd_rcu(sourcefd); else source = task_active_pid_ns(current); if (targetfd >= 0) target = pidns_by_fd_rcu(targetfd); else target = task_active_pid_ns(current); pid = find_pid_ns(pid_nr, source); result = pid_nr_ns(pid, target); if (result == 0) result = -ESRCH; rcu_read_unlock(); return result; } Eric