linux-kernel - Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

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: <22a505e0f094352b9cb883935fae9f494fb2cb0b.camel@kernel.crashing.org>
Date:   Mon, 11 Jun 2018 10:05:22 +1000
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Michael Schmitz <schmitzmic@...il.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Finn Thain <fthain@...egraphics.com.au>
Cc:     Andreas Schwab <schwab@...ux-m68k.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        linux-m68k <linux-m68k@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on
 unsupported hardware

On Sun, 2018-06-10 at 21:12 +1200, Michael Schmitz wrote:
> Hi Geert,

Top posting, sorry ...

We are painting that bike shed with way too many coats..

We can keep the existing definitions, stick a comment on them stating
"obsolete" and use new number if/when needed.

Ben.


> Am 10.06.2018 um 20:29 schrieb Geert Uytterhoeven:
> > Hi Finn,
> > 
> > On Sat, Jun 9, 2018 at 2:20 PM Finn Thain <fthain@...egraphics.com.au> wrote:
> > > > > > Is this enum used by any user space code? If so, perhaps rather
> > > > > > leave the PMU_68K_V1 in there to avoid upsetting that?
> > > > > 
> > > > > It also changes the value of PMU_68K_V2, which is an ABI break.
> > > > 
> > > > Yes, that's what I worry about - but do we know of any users of that
> > > > particular interface?
> > > 
> > > There is no ABI issue AFAIK. The value of pmu_kind is visible to userland
> > > only on powerpc. /dev/pmu and /proc/pmu/* do not exist on m68k. This patch
> > > series will make these UAPIs available on m68k, and for that reason I've
> > > chosen the value PMU_UNKNOWN for pmu_kind.
> > 
> > While /dev/pmu and /proc/pmu/* may not exist on m68k, definitions in
> > include/uapi/linux/pmu.h are part of the ABI, and cannot be changed or removed,
> > unless we are 100% sure there are no users.
> > 
> > If I would write a program interfacing with /dev/pmu and /proc/pmu/*, and
> > needing to check the PMU type, it would have a switch() statement with
> > all existing values defined in <linux/pmu.h>. So that would become broken
> > by your change.
> > 
> > Hence the enum is append-only.
> 
> The PMU type from pmu.h was never exposed to user space on m68k via 
> /proc/pmu/*, and /dev/pmu is used for ioctls to the PMU driver on 
> powerpc only (the 68k PMU driver doesn't have ioctl support). No way 
> that I can see for user space to make use of the PMU type definition 
> from pmu.h, so I suppose we can be sure there are no users.
> 
> The m68k PMU types cannot be said to be exposed on powerpc either (which 
> has ioctl support to interrogate the PMU type), as these only return 
> values up to PMU_KEYLARGO_BASED.
> 
> Applications like pbbuttonsd or pmud don't use the kernel PMU type at 
> all, but go straight to the PMU via the ADB bus to interrogate the 
> hardware type, so won't be affected either.
> 
> Is there any other way besides procfs and ioctl for user space to 
> interrogate the PMU type that I'm missing here?
> 
> (I understand that breaking the ABI should not be done as a rule, but 
> this may be a case where we can successfully argue the definitions were 
> never in use, so the rules may be bent a little).