linux-kernel - Re: [PATCH v1 4/4] misc: fastrpc: Add debugfs support for fastrpc

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: <2024112120-sandal-reach-2e09@gregkh>
Date: Thu, 21 Nov 2024 14:30:10 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Ekansh Gupta <quic_ekangupt@...cinc.com>
Cc: srinivas.kandagatla@...aro.org, linux-arm-msm@...r.kernel.org,
	quic_bkumar@...cinc.com, linux-kernel@...r.kernel.org,
	quic_chennak@...cinc.com, dri-devel@...ts.freedesktop.org,
	arnd@...db.de
Subject: Re: [PATCH v1 4/4] misc: fastrpc: Add debugfs support for fastrpc

On Thu, Nov 21, 2024 at 12:12:17PM +0530, Ekansh Gupta wrote:
> On 11/18/2024 7:32 PM, Greg KH wrote:
> > On Mon, Nov 18, 2024 at 02:10:46PM +0530, Ekansh Gupta wrote:
> >> +		/*
> >> +		 * Use HLOS process name, HLOS PID, fastrpc user TGID,
> >> +		 * domain_id in debugfs filename to create unique file name
> >> +		 */
> >> +		snprintf(debugfs_buf, size, "%.10s_%d_%d_%d",
> >> +			cur_comm, current->pid, fl->tgid, domain_id);
> >> +		fl->debugfs_file = debugfs_create_file(debugfs_buf, 0644,
> >> +				debugfs_dir, fl, &fastrpc_debugfs_fops);
> > Why are you saving the debugfs file?  What do you need to do with it
> > that you can't just delete the whole directory, or look up the name
> > again in the future when removing it?
> fl structure is specific to a process using fastrpc driver. The reason to save
> this debugfs file is to delete is when the process releases fastrpc device.
> If the file is not deleted, it might flood multiple files in debugfs directory.
> 
> As part of this change, only the file that is getting created by a process is
> getting removed when process is releasing device and I don't think we
> can clean up the whole directory at this point.
> 
> Do you suggest that looking up the name is a better approach that saving
> the file?

Yes.

> >> +}
> >> +
> >> +struct dentry *fastrpc_create_debugfs_dir(const char *name)
> >> +{
> >> +	return debugfs_create_dir(name, NULL);
> > At the root of debugfs?  Why is this function even needed?
> creating a dir named "fastrpc_adsp", "fastrpc_cdsp" etc. to create debugfs
> file for the processes using adsp, cdsp etc.

Then just call debugfs_create_dir() you do not need a wrapper function
for this.

thanks,

greg k-h