DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C431AC04B316
Date: Wed, 7 Jun 2017 14:36:57 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
        Kees Cook <keescook@chromium.org>,
        Roland McGrath <roland@hack.frob.com>,
        Al Viro <viro@ZenIV.linux.org.uk>, David Howells <dhowells@redhat.com>,
        "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Subject: Re: [PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock
Message-ID: <20170607123657.GA22199@redhat.com>
References: <877f0pym71.fsf@xmission.com>
 <20170606190338.28347-1-ebiederm@xmission.com>
 <20170606190338.28347-6-ebiederm@xmission.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170606190338.28347-6-ebiederm@xmission.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 714
Lines: 27

Hi Eric,

I'll try very much to read this series tomorrow, can't do this today...

On 06/06, Eric W. Biederman wrote:
>
> @@ -1380,13 +1380,6 @@ int do_prlimit(struct task_struct *tsk, unsigned int resource,
>  			return -EPERM;
>  	}
>  
> -	/* protect tsk->signal and tsk->sighand from disappearing */
> -	read_lock(&tasklist_lock);
> -	if (!tsk->sighand) {
> -		retval = -ESRCH;
> -		goto out;
> -	}

Yes, the comment is wrong.

However we do need read_lock(tasklist_lock) to access ->group_leader. And the
->sighand != NULL check ensures that ->group_leader is the valid pointer.

Also, update_rlimit_cpu() is not safe without tasklist / sighand-check.

We can probably change this code to rely on rcu.

Oleg.