Subject: Re: [PATCH] debugfs: Add check for module parameter name
To: Al Viro <viro@ZenIV.linux.org.uk>
References: <8cf113648f45ccbf304735fcc4487adecd5b7082.1505996056.git.arvind.yadav.cs@gmail.com>
 <20170921124438.GH32076@ZenIV.linux.org.uk>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
From: arvind <arvind.yadav.cs@gmail.com>
Message-ID: <59C3ED32.1070807@gmail.com>
Date: Thu, 21 Sep 2017 22:17:46 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.6.0
MIME-Version: 1.0
In-Reply-To: <20170921124438.GH32076@ZenIV.linux.org.uk>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1587
Lines: 45

Hi,

On Thursday 21 September 2017 06:14 PM, Al Viro wrote:
> On Thu, Sep 21, 2017 at 05:46:54PM +0530, Arvind Yadav wrote:
>> Here, start_creating() is calling by debugfs_create_dir()
>> and debugfs_create_automount(). driver can pass name as NULL in
>> debugfs_create_dir and debugfs_create_automount. So we need to
>> add check for 'name'.
> Huh?  "Driver can pass any kind of crap pointer when calling this
> function, so let's check if that crap happens to be NULL and bail
> out in that particular case"?  Or am I misreading that?
Your are correct.
>
> Do you have any in-tree examples, or is that about some out-of-tree
> code that needs to be saved from itself?
>
Please check "drivers/base/power/opp/debugfs.c"

static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
                                       struct opp_table *opp_table,
                                       struct dentry *pdentry)
{
         struct dentry *d;
         int i;
         char *name;

         for (i = 0; i < opp_table->regulator_count; i++) {
                 name = kasprintf(GFP_KERNEL, "supply-%d", i);

                 /* Create per-opp directory */
                 d = debugfs_create_dir(name, pdentry);

                 kfree(name);
.
.
.
}

kasprintf() can fail here and It can return NULL.
In this case,  We are passing NULL value to debugfs_create_dir().
I know, we will have to handle kasprintf() first instead of adding
NULL check in start_creating(). I have seen few driver where they have done
similar kind of implementation. Also I am adding check for kasprintf.

~arvind