linux-kernel - Re: x86, microcode: Conversion from sysdev class caused regression

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: <CAPXgP11mjyK7vJoQtsSf3kD-SYn6d_Y9feobY9KiiSYcFqOwwA@mail.gmail.com>
Date:	Wed, 11 Apr 2012 19:06:21 +0200
From:	Kay Sievers <kay@...y.org>
To:	Andreas Herrmann <andreas.herrmann3@....com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Petkov, Borislav" <Borislav.Petkov@....com>,
	linux-kernel@...r.kernel.org
Subject: Re: x86, microcode: Conversion from sysdev class caused regression

On Wed, Apr 11, 2012 at 18:38, Andreas Herrmann
<andreas.herrmann3@....com> wrote:

> The reason for the error is that subsys_interface_register() doesn't
> handle the return value of sif->add_dev (and there's also no unwinding
> of the interface registration). Instead subsys_interface_register
> always returns 0.

Which is the intended behaviour of 'subsystem interfaces' from the
driver-core perspective. It should not matter if one of a bunch of
devices do not 'like' this 'interface'. It is the same model as a
'driver', we do not cancel the link-in of a driver if one device does
not like the driver.

> This is different to what we had for
> sysdev_driver_register(). The latter properly handled return codes of
> drv->add() functions.

A 'subsystem interface' is a global instance per subsystem, not
something _all_ devices must agree on, to be registered. We can load
almost all drivers in the kernel, without even having a device around;
the 'interfaces' should not be that different in their model.

> See Boris' commit f4203e3032e5ae74c3e89df85a5a6d96022d0c49 (sysdev: Do
> not register with sysdev when erroring on add).

It seems to paper over a bug of the microcode driver, we should not do
that, or make such assumptions.

> I don't know whether the microcode driver is the only driver that
> is affected.

All interfaces with the same assumption of the microcode driver might,
but see below.

> What's your suggestion for a fix? Should subsys_interface_register()
> be fixed or should each affected driver be adapted?

All the drivers, if there are any more, who are as naive as the
microcode driver. :)

"Subsystem interfaces' users must not assume that they can blindly
unwind their own stuff, which they have never registered.
mc_device_remove() just goes ahead without any check and calls
sysfs_remove_group(), while it should check its own state of this
specific CPU it acts on.

I think a quick return in the microcode driver, for a device which has
no active interface state is the best solution here.

Thanks,
Kay
--
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/