linux-kernel - Re: [PATCH] android: binder: print error message on failure of creating proc file

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: <CANTT7qj+FNiVsvK=d3cgKHJXo8kjriVh3PcGnk4t+Fgb12==sg@mail.gmail.com>
Date: Fri, 12 Jul 2024 14:54:55 +0900
From: Leesoo Ahn <lsahn@...eel.net>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Leesoo Ahn <lsahn@...akecorp.com>, Arve Hjønnevåg <arve@...roid.com>, 
	Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>, 
	Joel Fernandes <joel@...lfernandes.org>, Christian Brauner <brauner@...nel.org>, 
	Carlos Llamas <cmllamas@...gle.com>, Suren Baghdasaryan <surenb@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] android: binder: print error message on failure of
 creating proc file

2024년 7월 12일 (금) 오후 2:40, Greg Kroah-Hartman <gregkh@...uxfoundation.org>님이 작성:
>
> On Fri, Jul 12, 2024 at 12:21:40PM +0900, Leesoo Ahn wrote:
> > It better prints out an error message to give more information if
> > calling debugfs_create_file() is failure and the return value has an
> > error code.
> >
> > Signed-off-by: Leesoo Ahn <lsahn@...eel.net>
> > ---
> >  drivers/android/binder.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index b21a7b246a0d..eb0fd1443d69 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -5673,6 +5673,7 @@ static int binder_open(struct inode *nodp, struct file *filp)
> >
> >       if (binder_debugfs_dir_entry_proc && !existing_pid) {
> >               char strbuf[11];
> > +             struct dentry *debugfs_entry;
> >
> >               snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
> >               /*
> > @@ -5681,10 +5682,19 @@ static int binder_open(struct inode *nodp, struct file *filp)
> >                * The printing code will anyway print all contexts for a given
> >                * PID so this is not a problem.
> >                */
> > -             proc->debugfs_entry = debugfs_create_file(strbuf, 0444,
> > +             debugfs_entry = debugfs_create_file(strbuf, 0444,
> >                       binder_debugfs_dir_entry_proc,
> >                       (void *)(unsigned long)proc->pid,
> >                       &proc_fops);
> > +             if (!IS_ERR(debugfs_entry)) {
> > +                     proc->debugfs_entry = debugfs_entry;
> > +             } else {
> > +                     int error;
> > +
> > +                     error = PTR_ERR(debugfs_entry);
> > +                     pr_warn("Unable to create file %s in debugfs (error %d)\n",
> > +                             strbuf, error);
>
> Even if we wanted to warn about this (hint, you don't, see previous
> response), this way to check is incorrect and will fail if debugfs is
> not enabled, which you don't want to have happen.
>
> So I'm guessing you did not test this with that config option disabled?

Oh, I haven't thought about this and just figured out it would work weird.
Thank you for mentioning it.

>
> thanks,
>
> greg k-h