From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752875AbdJaJqm (ORCPT ); Tue, 31 Oct 2017 05:46:42 -0400 Received: from mx2.suse.de ([195.135.220.15]:37584 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751378AbdJaJqk (ORCPT ); Tue, 31 Oct 2017 05:46:40 -0400 Subject: Re: [PATCH RFC v2 4/4] mm/mempolicy: add nodes_empty check in SYSC_migrate_pages To: Yisheng Xie , akpm@linux-foundation.org, mhocko@suse.com, mingo@kernel.org, rientjes@google.com, n-horiguchi@ah.jp.nec.com, salls@cs.ucsb.edu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, tanxiaojun@huawei.com, linux-api@vger.kernel.org, Andi Kleen , Christoph Lameter References: <1509099265-30868-1-git-send-email-xieyisheng1@huawei.com> <1509099265-30868-5-git-send-email-xieyisheng1@huawei.com> From: Vlastimil Babka Message-ID: Date: Tue, 31 Oct 2017 10:46:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1509099265-30868-5-git-send-email-xieyisheng1@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +CC Andi and Christoph On 10/27/2017 12:14 PM, Yisheng Xie wrote: > As manpage of migrate_pages, the errno should be set to EINVAL when none > of the specified nodes contain memory. However, when new_nodes is null, > i.e. the specified nodes also do not have memory, as the following case: > > new_nodes = 0; > old_nodes = 0xf; > ret = migrate_pages(pid, old_nodes, new_nodes, MAX); > > The ret will be 0 and no errno is set. > > This patch is to add nodes_empty check to fix above case. Hmm, I think we have a bigger problem than "empty set is a subset of anything" here. The existing checks are: task_nodes = cpuset_mems_allowed(task); /* Is the user allowed to access the target nodes? */ if (!nodes_subset(*new, task_nodes) && !capable(CAP_SYS_NICE)) { err = -EPERM; goto out_put; } if (!nodes_subset(*new, node_states[N_MEMORY])) { err = -EINVAL; goto out_put; } And manpage says: EINVAL The value specified by maxnode exceeds a kernel-imposed limit. Or, old_nodes or new_nodes specifies one or more node IDs that are greater than the maximum supported node ID. *Or, none of the node IDs specified by new_nodes are on-line and allowed by the process's current cpuset context, or none of the specified nodes contain memory.* EPERM Insufficient privilege (CAP_SYS_NICE) to move pages of the process specified by pid, or insufficient privilege (CAP_SYS_NICE) to access the specified target nodes. - it says "none ... are allowed", but checking for subset means we check if "all ... are allowed". Shouldn't we be checking for a non-empty intersection? - there doesn't seem to be any EINVAL check for "process's current cpuset context", there's just an EPERM check for "target process's cpuset context". > > Signed-off-by: Yisheng Xie > --- > mm/mempolicy.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 8798ecb..58352cc 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1402,6 +1402,11 @@ static int copy_nodes_to_user(unsigned long __user *mask, unsigned long maxnode, > if (err) > goto out; > > + if (nodes_empty(*new)) { > + err = -EINVAL; > + goto out; > + } > + > /* Find the mm_struct */ > rcu_read_lock(); > task = pid ? find_task_by_vpid(pid) : current; >