linux-kernel - Re: [PATCH 1/2] drivers: base: Warn if driver name is not present

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: <20200608105756.GB295073@kroah.com>
Date:   Mon, 8 Jun 2020 12:57:56 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     matthias.bgg@...nel.org
Cc:     rafael@...nel.org, linux-kernel@...r.kernel.org,
        gene.chen.richtek@...il.com, lee.jones@...aro.org,
        Matthias Brugger <mbrugger@...e.com>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH 1/2] drivers: base: Warn if driver name is not present

On Mon, Jun 08, 2020 at 11:52:16AM +0200, matthias.bgg@...nel.org wrote:
> From: Matthias Brugger <mbrugger@...e.com>
> 
> If we pass a driver without a name, we end up in a NULL pointer
> derefernce.

That's a very good reason not to have a driver without a name :)

What in-kernel driver does this?

> Check for the name before trying to register the driver.
> As we don't have a driver name to point to in the error message, we dump
> the call stack to make it easier to detect the buggy driver.
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Signed-off-by: Matthias Brugger <mbrugger@...e.com>
> ---
>  drivers/base/driver.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 57c68769e157..40fba959c140 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -149,6 +149,12 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> +	if (!drv->name) {
> +		pr_err("Driver has no name.\n");
> +		dump_stack();
> +		return -EINVAL;

Ick, no, an oops-traceback for doing something dumb like this should be
all that we need, right?

How "hardened" do we need to make internal apis anyway?  What's the odds
that if this does trigger, the driver author would even notice it?

thanks,

greg k-h