All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Fei Zhang <zhangfeionline@gmail.com>
Cc: rafael@kernel.org, linux-kernel@vger.kernel.org,
	songmuchun@bytedance.com
Subject: Re: [PATCH] driver core: Fix possible use after free on name
Date: Mon, 6 Apr 2020 07:41:10 +0200	[thread overview]
Message-ID: <20200406054110.GA1638548@kroah.com> (raw)
In-Reply-To: <CAC_binJNLLxfOm0W2TuVTJZxJRTZTvPPocSDNQMU=21XO37oZg@mail.gmail.com>

On Mon, Apr 06, 2020 at 01:33:03PM +0800, Fei Zhang wrote:
> Dear Greg,
> 
> Please refer to below for the crash. If you are fine with it, I will
> submit another patch with correcting the error mentioned.
> 
> When writing a kernel driver module, we may use it like this:
> 
> static int frv_init(int index)
> {
> ...
> 	char name [128]={0};
> 	sprintf(name,"test_%d",index);
> 	mount_dev_p->pci_class =class_create(THIS_MODULE,name);
> 	classdev = device_create(mount_dev_p->pci_class, NULL, devno,
> 	NULL, "PCIE_%x",index);
> ...
> }
> 
> static void frv_exit(void)
> {
> ...
> 	device_destroy(mount_dev_p->pci_class,mount_dev_p->devno);
> 	class_destroy(mount_dev_p->pci_class );
> ...
> }
> 
> But when we remove the module, a crash occurres when calling
> device_destroy.
> 
> Call Trace:
>  vsnprintf+0x2b2/0x4e0
>  add_uevent_var+0x7d/0x110
>  kobject_uevent_env+0x23f/0x770
>  kobject_uevent+0xb/0x10
>  device_del+0x23b/0x360
>  device_unregister+0x1a/0x60
>  device_destroy+0x3c/0x50
> 
> I traced the code and found that an invalid local variable was called
> in "kobject_uevent_env()", and triggered further crash as followed:
> 
> kobject_uevent_env(...)
> {
> ...
> struct kset_uevent_ops *uevent_ops;
> uevent_ops = kset->uevent_ops;
> subsystem = uevent_ops->name(kset, kobj);
> add_uevent_var(env, "SUBSYSTEM=%s", subsystem);
> ...
> }
> 
> What is the "subsystem" value, continue to track.
> 
> static const struct kset_uevent_ops device_uevent_ops = {
> 	.name =		dev_uevent_name,
> };
> 
> static const char *dev_uevent_name(struct kset *kset, struct kobject *kobj)
> {
> 	struct device *dev = kobj_to_dev(kobj);
> 	if (dev->class)
> 		return dev->class->name;
> 	return NULL;
> }
> 
> Everything becomes clear: "class->name" and "subsystem" value is local
> variable array address of start module function inside "char name [128]".
> Used released local variable in device_destroy's kobject_uevent_env, an
> error occurred.

Please do not top-post.

Anyway, yes, that does seem to be a semi-valid way of creating a class,
does anyone currently do that in the kernel tree today?  Typically
creating classes is rare, and do not have a "dynamic name" like your
example above, because that is not what a class is for.

So while the idea is good to solve this, I would like to go back to your
example to find out why you are doing this in the first place, as that
does not seem to be the way to use the driver model correctly.

thanks,

greg k-h

  reply	other threads:[~2020-04-06  5:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-05 16:05 [PATCH] driver core: Fix possible use after free on name zhangfeionline
2020-04-05 16:40 ` Greg KH
2020-04-06  5:33   ` Fei Zhang
2020-04-06  5:41     ` Greg KH [this message]
2020-04-06  7:40       ` Fei Zhang
2020-04-06  8:28         ` Greg KH
2020-04-06 10:42           ` [External] " 宋牧春
2020-04-06 11:16             ` Greg KH
     [not found]               ` <CAC_bin+tzPeHX2bAz+0hY+qKsBn4-vMuqFvYvW05bDGv32SzEw@mail.gmail.com>
2020-04-07 15:01                 ` Greg KH
2020-04-06 11:04   ` 宋牧春

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200406054110.GA1638548@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=songmuchun@bytedance.com \
    --cc=zhangfeionline@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.