linux-kernel - Re: [RFC v2] perf: Rewrite core context handling

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: <8e8202e2-6e28-61f0-b6b1-d3f782ba90f7@amd.com>
Date:   Tue, 23 Aug 2022 10:27:07 +0530
From:   Ravi Bangoria <ravi.bangoria@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     acme@...nel.org, alexander.shishkin@...ux.intel.com,
        jolsa@...hat.com, namhyung@...nel.org, songliubraving@...com,
        eranian@...gle.com, alexey.budankov@...ux.intel.com,
        ak@...ux.intel.com, mark.rutland@....com, megha.dey@...el.com,
        frederic@...nel.org, maddy@...ux.ibm.com, irogers@...gle.com,
        kim.phillips@....com, linux-kernel@...r.kernel.org,
        santosh.shukla@....com, ravi.bangoria@....com
Subject: Re: [RFC v2] perf: Rewrite core context handling

On 22-Aug-22 10:22 PM, Peter Zijlstra wrote:
> On Tue, Aug 02, 2022 at 11:41:42AM +0530, Ravi Bangoria wrote:
> 
>>> @@ -12358,58 +12374,14 @@ SYSCALL_DEFINE5(perf_event_open,
>>>  			goto err_context;
>>>  	}
>>>  
>>> -	event_file = anon_inode_getfile("[perf_event]", &perf_fops, event, f_flags);
>>> -	if (IS_ERR(event_file)) {
>>> -		err = PTR_ERR(event_file);
>>> -		event_file = NULL;
>>> -		goto err_context;
>>> -	}
>>> -
>>> -	if (task) {
>>> -		err = down_read_interruptible(&task->signal->exec_update_lock);
>>> -		if (err)
>>> -			goto err_file;
>>> -
>>> -		/*
>>> -		 * We must hold exec_update_lock across this and any potential
>>> -		 * perf_install_in_context() call for this new event to
>>> -		 * serialize against exec() altering our credentials (and the
>>> -		 * perf_event_exit_task() that could imply).
>>> -		 */
>>> -		err = -EACCES;
>>> -		if (!perf_check_permission(&attr, task))
>>> -			goto err_cred;
>>> -	}
>>> -
>>> -	if (ctx->task == TASK_TOMBSTONE) {
>>> -		err = -ESRCH;
>>> -		goto err_locked;
>>> -	}
>>
>> I think we need to keep (ctx->task == TASK_TOMBSTONE) check?
> 
> I think so too; in fact the code I have still has it, perhaps it was
> there write before this patch?
> 
>>> -
>>>  	if (!perf_event_validate_size(event)) {
>>>  		err = -E2BIG;
>>> -		goto err_locked;
>>> -	}
>>> -
>>> -	if (!task) {
>>> -		/*
>>> -		 * Check if the @cpu we're creating an event for is online.
>>> -		 *
>>> -		 * We use the perf_cpu_context::ctx::mutex to serialize against
>>> -		 * the hotplug notifiers. See perf_event_{init,exit}_cpu().
>>> -		 */
>>> -		struct perf_cpu_context *cpuctx =
>>> -			container_of(ctx, struct perf_cpu_context, ctx);
>>> -
>>> -		if (!cpuctx->online) {
>>> -			err = -ENODEV;
>>> -			goto err_locked;
>>> -		}
>>> +		goto err_context;
>>
>> Why did you remove this hunk? We should confirm whether cpu is online or not
>> before creating event. No?
> 
> Idem.
> 
> Perhaps it is best if we look at the end result of all these patches
> combined and then I'll fold the lot if we're in agreement and then we
> can forget about these intermediate steps.

Let me accumulate all these changes, rebase to v6.0-rc2 and send RFC v3.

Thanks,
Ravi