linux-kernel - Re: [External] Re: [RFC] mm: add new syscall pidfd_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <582cf257-bc0d-c96e-e72e-9164cff4fce1@bytedance.com>
Date:   Wed, 12 Oct 2022 15:55:44 +0800
From:   Zhongkun He <hezhongkun.hzk@...edance.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     corbet@....net, akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-doc@...r.kernel.org, wuyun.abel@...edance.com
Subject: Re: [External] Re: [RFC] mm: add new syscall pidfd_set_mempolicy()

Hi  michal, thanks for your reply and suggestiones.

> Please add some explanation why the cpuset interface is not usable for
> that usecase.
OK.

>> To solve the issue, this patch introduces a new syscall
>> pidfd_set_mempolicy(2).  it sets the NUMA memory policy of the thread
>> specified in pidfd.
>>
>> In current process context there is no locking because only the process
>> accesses its own memory policy, so task_work is used in
>> pidfd_set_mempolicy() to update the mempolicy of the process specified
>> in pidfd, avoid using locks and race conditions.
> 
> Why cannot you alter kernel_set_mempolicy (and do_set_mempolicy) to
> accept a task rather than operate on current?

I have tried it before this patch, but I found a problem.The allocation 
and update of mempolicy are in the current context, so it is not 
protected by any lock.But when the mempolicy is modified by other 
processes, the race condition appears.
Say something like the following

	pidfd_set_mempolicy	     target task stack
				       alloc_pages
					mpol = get_task_policy;
	 task_lock(task);
	 old = task->mempolicy;
	 task->mempolicy = new;
	 task_unlock(task);
	 mpol_put(old);			
					page = __alloc_pages(mpol);
There is a situation that when the old mempolicy is released, the target 
task is still using the policy.It would be better if there are 
suggestions on this case.

> I have to really say that I dislike the task_work approach because it
> detaches the syscall from the actual operation and the caller simply
> doesn't know when the operation has been completed.

I agree with you.This is indeed a problem.

> Please also describe the security model.got it.