linux-kernel - Re: [PATCH v2] pids: introduce find_get_task_by

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: <20171030094436.GA3141@rapoport-lnx>
Date:   Mon, 30 Oct 2017 11:44:37 +0200
From:   Mike Rapoport <rppt@...ux.vnet.ibm.com>
To:     Balbir Singh <bsingharora@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Darren Hart <dvhart@...radead.org>,
        Oleg Nesterov <oleg@...hat.com>, linux-mm <linux-mm@...ck.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] pids: introduce find_get_task_by_vpid helper

On Mon, Oct 30, 2017 at 07:51:42PM +1100, Balbir Singh wrote:
> On Sat, Oct 28, 2017 at 4:52 AM, Mike Rapoport <rppt@...ux.vnet.ibm.com> wrote:
> > There are several functions that do find_task_by_vpid() followed by
> > get_task_struct(). We can use a helper function instead.
> >
> > Signed-off-by: Mike Rapoport <rppt@...ux.vnet.ibm.com>
> > ---
> 
> I did a quick grep and found other similar patterns in

(reordered the file list a bit)

> kernel/events/core.c,
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c,
> mm/mempolicy.c,

Those and mm/migrate.c indeed have a similar pattern, but they all do

	task = pid ? find_task_by_vpid(pid) : current;

And I don't see an elegant way to use find_get_task_by_vpid() in this case.

> kernel/kcmp.c,

kcmp gets both tasks between rcu_read_lock/unlock and I think it's better
to keep it this way.

> kernel/sys.c,

There is no get_task_struct() after find_task_by_vpid(), unless I've missed
something

> kernel/time/posix-cpu-timers.c,

Here the task is selected with more complex logic than just
find_task_by_vpid() 

> mm/process_vm_access.c,

Converted in the patch

> security/yama/yama_lsm.c,
> arch/ia64/kernel/perfmon.c

I've missed these two, indeed.

The arch/ia64/kernel/perfmon.c even still uses read_lock(&tasklist) rather
than rcu_read_lock()...
 
> Balbir Singh.
> 

-- 
Sincerely yours,
Mike.