linux-kernel - Re: [PATCH 1/4] module: implement module_inhibit

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]

Date:	Tue, 25 Sep 2007 14:38:38 +1000
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Tejun Heo <htejun@...il.com>
Cc:	Jonathan Corbet <corbet@....net>, ebiederm@...ssion.com,
	cornelia.huck@...ibm.com, greg@...ah.com,
	stern@...land.harvard.edu, kay.sievers@...y.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] module: implement module_inhibit_unload()

On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
> Rusty Russell wrote:
> > As stated you cannot protect arbitrary code this way, as you are trying
> > to do.  I do not think you've broken any of the current code, but I
> > cannot tell.  You're certainly going to surprise unsuspecting future
> > authors.
> 
> Can you elaborate a bit?  Why can't it protect the code?

Because you don't know what that code does.  After all, it's assumed
that module code doesn't get called after exit and you're deliberately
violating that assumption.

> > Can you really not figure out the module owner of the sysfs entry to inc
> > its use count during this procedure?  (__module_get()).
> 
> I can but I don't think it's worth the effort.  It will involve passing
> @owner parameter down through kobject to sysfs but the path is pretty
> obscure and thus difficult to test.

Have you tested that *this* path works?  Let's take your first change as
an example:

+       mutex_lock(&gdev->reg_mutex);
+       __ccwgroup_remove_symlinks(gdev);
+       device_unregister(dev);
+       mutex_unlock(&gdev->reg_mutex);

Now, are you sure that calling cleanup_ccwgroup just after
device_unregister() works?

static void __exit
cleanup_ccwgroup (void)
{
	bus_unregister (&ccwgroup_bus_type);
}

> I think it's too much work for the
> users of the API and it will be easy to pass the wrong @owner and go
> unnoticed.

But your shortcut insists that all module authors be aware that
functions can be running after exit() is called.  That's a recipe for
instability and disaster.

Rusty.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/